Clear effect tags from a fiber that suspends in non-concurrent mode
Even though we commit the fiber in an incomplete state, we shouldn't fire any lifecycles or effects. We already did this for classes, but now with useEffect, the same is needed for other types of work, too.
Andrew Clark committed
Oct 23, 2018 at 00:41 UTC
acb48996373ee0cbf1b2462972cd7644862d770e
2 files changed
+52
-5
packages/react-reconciler/src/ReactFiberUnwindWork.js
+6
-5
@@ -248,15 +248,16 @@ function throwException(
248
);
249
sourceFiber.effectTag &= ~Incomplete;
250
251
+ // We're going to commit this fiber even though it didn't complete.
252
+ // But we shouldn't call any lifecycle methods or callbacks. Remove
253
+ // all lifecycle effect tags.
254
+ sourceFiber.effectTag &= ~LifecycleEffectMask;
255
+
256
if (sourceFiber.tag === ClassComponent) {
252
- // We're going to commit this fiber even though it didn't complete.
253
- // But we shouldn't call any lifecycle methods or callbacks. Remove
254
- // all lifecycle effect tags.
255
- sourceFiber.effectTag &= ~LifecycleEffectMask;
257
const current = sourceFiber.alternate;
258
if (current === null) {
259
// This is a new mount. Change the tag so it's not mistaken for a
259
- // completed component. For example, we should not call
260
+ // completed class component. For example, we should not call
261
// componentWillUnmount if it is deleted.
262
sourceFiber.tag = IncompleteClassComponent;
263
}
packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js
+46
@@ -17,6 +17,7 @@ describe('ReactSuspense', () => {
17
ReactFeatureFlags = require('shared/ReactFeatureFlags');
18
ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false;
19
ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false;
20
+ ReactFeatureFlags.enableHooks = true;
21
React = require('react');
22
ReactTestRenderer = require('react-test-renderer');
23
// JestReact = require('jest-react');
@@ -526,6 +527,51 @@ describe('ReactSuspense', () => {
527
expect(root).toMatchRenderedOutput('B');
528
});
529
530
+ it('suspends in a component that also contains useEffect', () => {
531
+ const {useLayoutEffect} = React;
532
+
533
+ function AsyncTextWithEffect(props) {
534
+ const text = props.text;
535
+
536
+ useLayoutEffect(
537
+ () => {
538
+ ReactTestRenderer.unstable_yield('Did commit: ' + text);
539
+ },
540
+ [text],
541
+ );
542
+
543
+ try {
544
+ TextResource.read([props.text, props.ms]);
545
+ ReactTestRenderer.unstable_yield(text);
546
+ return text;
547
+ } catch (promise) {
548
+ if (typeof promise.then === 'function') {
549
+ ReactTestRenderer.unstable_yield(`Suspend! [${text}]`);
550
+ } else {
551
+ ReactTestRenderer.unstable_yield(`Error! [${text}]`);
552
+ }
553
+ throw promise;
554
+ }
555
+ }
556
+
557
+ function App({text}) {
558
+ return (
559
+ <Suspense fallback={<Text text="Loading..." />}>
560
+ <AsyncTextWithEffect text={text} ms={100} />
561
+ </Suspense>
562
+ );
563
+ }
564
+
565
+ ReactTestRenderer.create(<App text="A" />);
566
+ expect(ReactTestRenderer).toHaveYielded(['Suspend! [A]', 'Loading...']);
567
+ jest.advanceTimersByTime(500);
568
+ expect(ReactTestRenderer).toHaveYielded([
569
+ 'Promise resolved [A]',
570
+ 'A',
571
+ 'Did commit: A',
572
+ ]);
573
+ });
574
+
575
it('retries when an update is scheduled on a timed out tree', () => {
576
let instance;
577
class Stateful extends React.Component {