Fix lazy() with defaultProps (#14112)
* Resolve defaultProps for Lazy components * Make test fail again * Undo the partial fix * Make test output more compact * Add a separate failing test for sync mode * Clean up tests * Add another update to both tests * Resolve props for commit phase lifecycles * Resolve prevProps for begin phase lifecycles * Resolve prevProps for pre-commit lifecycles * Only resolve props if element type differs * Fix Flow * Don't set instance.props/state during commit phase This is an optimization. I'm not sure it's entirely safe. It's probably worth running internal tests and see if we can ever trigger a case where they're different. This can mess with resuming. * Keep setting instance.props/state before unmounting This reverts part of the previous commit. It broke a test that verifies we use current props in componentWillUnmount if the fiber unmounts due to an error.
Dan Abramov committed
Nov 6, 2018 at 19:54 UTC
f777d196e0943b3975c762e15e35b319a9b85902
5 files changed
+223
-27
packages/react-reconciler/src/ReactFiberBeginWork.js
+4
-16
@@ -103,7 +103,10 @@ import {
103
resumeMountClassInstance,
104
updateClassInstance,
105
} from './ReactFiberClassComponent';
106
-import {readLazyComponentType} from './ReactFiberLazyComponent';
106
+import {
107
+ readLazyComponentType,
108
+ resolveDefaultProps,
109
+} from './ReactFiberLazyComponent';
110
import {
111
resolveLazyComponentTag,
112
createFiberFromTypeAndProps,
@@ -729,21 +732,6 @@ function updateHostText(current, workInProgress) {
732
return null;
733
}
734
732
-function resolveDefaultProps(Component, baseProps) {
733
- if (Component && Component.defaultProps) {
734
- // Resolve default props. Taken from ReactElement
735
- const props = Object.assign({}, baseProps);
736
- const defaultProps = Component.defaultProps;
737
- for (let propName in defaultProps) {
738
- if (props[propName] === undefined) {
739
- props[propName] = defaultProps[propName];
740
- }
741
- }
742
- return props;
743
- }
744
- return baseProps;
745
-}
746
-
735
function mountLazyComponent(
736
_current,
737
workInProgress,
packages/react-reconciler/src/ReactFiberClassComponent.js
+5
-1
@@ -28,6 +28,7 @@ import warningWithoutStack from 'shared/warningWithoutStack';
28
import {REACT_CONTEXT_TYPE} from 'shared/ReactSymbols';
29
30
import {startPhaseTimer, stopPhaseTimer} from './ReactDebugFiberPerf';
31
+import {resolveDefaultProps} from './ReactFiberLazyComponent';
32
import {StrictMode} from './ReactTypeOfMode';
33
34
import {
@@ -987,7 +988,10 @@ function updateClassInstance(
988
const instance = workInProgress.stateNode;
989
990
const oldProps = workInProgress.memoizedProps;
990
- instance.props = oldProps;
991
+ instance.props =
992
+ workInProgress.type === workInProgress.elementType
993
+ ? oldProps
994
+ : resolveDefaultProps(workInProgress.type, oldProps);
995
996
const oldContext = instance.context;
997
const contextType = ctor.contextType;
packages/react-reconciler/src/ReactFiberCommitWork.js
+20
-10
@@ -60,6 +60,7 @@ import {onCommitUnmount} from './ReactFiberDevToolsHook';
60
import {startPhaseTimer, stopPhaseTimer} from './ReactDebugFiberPerf';
61
import {getStackByFiberInDevAndProd} from './ReactCurrentFiber';
62
import {logCapturedError} from './ReactFiberErrorLogger';
63
+import {resolveDefaultProps} from './ReactFiberLazyComponent';
64
import {getCommitTime} from './ReactProfilerTimer';
65
import {commitUpdateQueue} from './ReactUpdateQueue';
66
import {
@@ -228,10 +229,13 @@ function commitBeforeMutationLifeCycles(
229
const prevState = current.memoizedState;
230
startPhaseTimer(finishedWork, 'getSnapshotBeforeUpdate');
231
const instance = finishedWork.stateNode;
231
- instance.props = finishedWork.memoizedProps;
232
- instance.state = finishedWork.memoizedState;
232
+ // We could update instance props and state here,
233
+ // but instead we rely on them being set during last render.
234
+ // TODO: revisit this when we implement resuming.
235
const snapshot = instance.getSnapshotBeforeUpdate(
234
- prevProps,
236
+ finishedWork.elementType === finishedWork.type
237
+ ? prevProps
238
+ : resolveDefaultProps(finishedWork.type, prevProps),
239
prevState,
240
);
241
if (__DEV__) {
@@ -345,16 +349,21 @@ function commitLifeCycles(
349
if (finishedWork.effectTag & Update) {
350
if (current === null) {
351
startPhaseTimer(finishedWork, 'componentDidMount');
348
- instance.props = finishedWork.memoizedProps;
349
- instance.state = finishedWork.memoizedState;
352
+ // We could update instance props and state here,
353
+ // but instead we rely on them being set during last render.
354
+ // TODO: revisit this when we implement resuming.
355
instance.componentDidMount();
356
stopPhaseTimer();
357
} else {
353
- const prevProps = current.memoizedProps;
358
+ const prevProps =
359
+ finishedWork.elementType === finishedWork.type
360
+ ? current.memoizedProps
361
+ : resolveDefaultProps(finishedWork.type, current.memoizedProps);
362
const prevState = current.memoizedState;
363
startPhaseTimer(finishedWork, 'componentDidUpdate');
356
- instance.props = finishedWork.memoizedProps;
357
- instance.state = finishedWork.memoizedState;
364
+ // We could update instance props and state here,
365
+ // but instead we rely on them being set during last render.
366
+ // TODO: revisit this when we implement resuming.
367
instance.componentDidUpdate(
368
prevProps,
369
prevState,
@@ -365,8 +374,9 @@ function commitLifeCycles(
374
}
375
const updateQueue = finishedWork.updateQueue;
376
if (updateQueue !== null) {
368
- instance.props = finishedWork.memoizedProps;
369
- instance.state = finishedWork.memoizedState;
377
+ // We could update instance props and state here,
378
+ // but instead we rely on them being set during last render.
379
+ // TODO: revisit this when we implement resuming.
380
commitUpdateQueue(
381
finishedWork,
382
updateQueue,
packages/react-reconciler/src/ReactFiberLazyComponent.js
+15
@@ -12,6 +12,21 @@ import type {LazyComponent, Thenable} from 'shared/ReactLazyComponent';
12
import {Resolved, Rejected, Pending} from 'shared/ReactLazyComponent';
13
import warning from 'shared/warning';
14
15
+export function resolveDefaultProps(Component: any, baseProps: Object): Object {
16
+ if (Component && Component.defaultProps) {
17
+ // Resolve default props. Taken from ReactElement
18
+ const props = Object.assign({}, baseProps);
19
+ const defaultProps = Component.defaultProps;
20
+ for (let propName in defaultProps) {
21
+ if (props[propName] === undefined) {
22
+ props[propName] = defaultProps[propName];
23
+ }
24
+ }
25
+ return props;
26
+ }
27
+ return baseProps;
28
+}
29
+
30
export function readLazyComponentType<T>(lazyComponent: LazyComponent<T>): T {
31
const status = lazyComponent._status;
32
const result = lazyComponent._result;
packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js
+179
@@ -282,6 +282,185 @@ describe('ReactLazy', () => {
282
expect(root).toMatchRenderedOutput('SiblingB');
283
});
284
285
+ it('sets defaultProps for modern lifecycles', async () => {
286
+ class C extends React.Component {
287
+ static defaultProps = {text: 'A'};
288
+ state = {};
289
+
290
+ static getDerivedStateFromProps(props) {
291
+ ReactTestRenderer.unstable_yield(
292
+ `getDerivedStateFromProps: ${props.text}`,
293
+ );
294
+ return null;
295
+ }
296
+
297
+ constructor(props) {
298
+ super(props);
299
+ ReactTestRenderer.unstable_yield(`constructor: ${this.props.text}`);
300
+ }
301
+
302
+ componentDidMount() {
303
+ ReactTestRenderer.unstable_yield(
304
+ `componentDidMount: ${this.props.text}`,
305
+ );
306
+ }
307
+
308
+ componentDidUpdate(prevProps) {
309
+ ReactTestRenderer.unstable_yield(
310
+ `componentDidUpdate: ${prevProps.text} -> ${this.props.text}`,
311
+ );
312
+ }
313
+
314
+ componentWillUnmount() {
315
+ ReactTestRenderer.unstable_yield(
316
+ `componentWillUnmount: ${this.props.text}`,
317
+ );
318
+ }
319
+
320
+ shouldComponentUpdate(nextProps) {
321
+ ReactTestRenderer.unstable_yield(
322
+ `shouldComponentUpdate: ${this.props.text} -> ${nextProps.text}`,
323
+ );
324
+ return true;
325
+ }
326
+
327
+ getSnapshotBeforeUpdate(prevProps) {
328
+ ReactTestRenderer.unstable_yield(
329
+ `getSnapshotBeforeUpdate: ${prevProps.text} -> ${this.props.text}`,
330
+ );
331
+ return null;
332
+ }
333
+
334
+ render() {
335
+ return <Text text={this.props.text + this.props.num} />;
336
+ }
337
+ }
338
+
339
+ const LazyClass = lazy(() => fakeImport(C));
340
+
341
+ const root = ReactTestRenderer.create(
342
+ <Suspense fallback={<Text text="Loading..." />}>
343
+ <LazyClass num={1} />
344
+ </Suspense>,
345
+ {
346
+ unstable_isConcurrent: true,
347
+ },
348
+ );
349
+
350
+ expect(root).toFlushAndYield(['Loading...']);
351
+ expect(root).toMatchRenderedOutput(null);
352
+
353
+ await Promise.resolve();
354
+
355
+ expect(root).toFlushAndYield([
356
+ 'constructor: A',
357
+ 'getDerivedStateFromProps: A',
358
+ 'A1',
359
+ 'componentDidMount: A',
360
+ ]);
361
+
362
+ root.update(
363
+ <Suspense fallback={<Text text="Loading..." />}>
364
+ <LazyClass num={2} />
365
+ </Suspense>,
366
+ );
367
+ expect(root).toFlushAndYield([
368
+ 'getDerivedStateFromProps: A',
369
+ 'shouldComponentUpdate: A -> A',
370
+ 'A2',
371
+ 'getSnapshotBeforeUpdate: A -> A',
372
+ 'componentDidUpdate: A -> A',
373
+ ]);
374
+ expect(root).toMatchRenderedOutput('A2');
375
+
376
+ root.update(
377
+ <Suspense fallback={<Text text="Loading..." />}>
378
+ <LazyClass num={3} />
379
+ </Suspense>,
380
+ );
381
+ expect(root).toFlushAndYield([
382
+ 'getDerivedStateFromProps: A',
383
+ 'shouldComponentUpdate: A -> A',
384
+ 'A3',
385
+ 'getSnapshotBeforeUpdate: A -> A',
386
+ 'componentDidUpdate: A -> A',
387
+ ]);
388
+ expect(root).toMatchRenderedOutput('A3');
389
+ });
390
+
391
+ it('sets defaultProps for legacy lifecycles', async () => {
392
+ class C extends React.Component {
393
+ static defaultProps = {text: 'A'};
394
+ state = {};
395
+
396
+ UNSAFE_componentWillMount() {
397
+ ReactTestRenderer.unstable_yield(
398
+ `UNSAFE_componentWillMount: ${this.props.text}`,
399
+ );
400
+ }
401
+
402
+ UNSAFE_componentWillUpdate(nextProps) {
403
+ ReactTestRenderer.unstable_yield(
404
+ `UNSAFE_componentWillUpdate: ${this.props.text} -> ${nextProps.text}`,
405
+ );
406
+ }
407
+
408
+ UNSAFE_componentWillReceiveProps(nextProps) {
409
+ ReactTestRenderer.unstable_yield(
410
+ `UNSAFE_componentWillReceiveProps: ${this.props.text} -> ${
411
+ nextProps.text
412
+ }`,
413
+ );
414
+ }
415
+
416
+ render() {
417
+ return <Text text={this.props.text + this.props.num} />;
418
+ }
419
+ }
420
+
421
+ const LazyClass = lazy(() => fakeImport(C));
422
+
423
+ const root = ReactTestRenderer.create(
424
+ <Suspense fallback={<Text text="Loading..." />}>
425
+ <LazyClass num={1} />
426
+ </Suspense>,
427
+ );
428
+
429
+ expect(ReactTestRenderer).toHaveYielded(['Loading...']);
430
+ expect(root).toFlushAndYield([]);
431
+ expect(root).toMatchRenderedOutput('Loading...');
432
+
433
+ await Promise.resolve();
434
+
435
+ root.update(
436
+ <Suspense fallback={<Text text="Loading..." />}>
437
+ <LazyClass num={2} />
438
+ </Suspense>,
439
+ );
440
+ expect(ReactTestRenderer).toHaveYielded([
441
+ 'UNSAFE_componentWillMount: A',
442
+ 'A1',
443
+ 'UNSAFE_componentWillReceiveProps: A -> A',
444
+ 'UNSAFE_componentWillUpdate: A -> A',
445
+ 'A2',
446
+ ]);
447
+ expect(root).toFlushAndYield([]);
448
+ expect(root).toMatchRenderedOutput('A2');
449
+
450
+ root.update(
451
+ <Suspense fallback={<Text text="Loading..." />}>
452
+ <LazyClass num={3} />
453
+ </Suspense>,
454
+ );
455
+ expect(ReactTestRenderer).toHaveYielded([
456
+ 'UNSAFE_componentWillReceiveProps: A -> A',
457
+ 'UNSAFE_componentWillUpdate: A -> A',
458
+ 'A3',
459
+ ]);
460
+ expect(root).toFlushAndYield([]);
461
+ expect(root).toMatchRenderedOutput('A3');
462
+ });
463
+
464
it('includes lazy-loaded component in warning stack', async () => {
465
const LazyFoo = lazy(() => {
466
ReactTestRenderer.unstable_yield('Started loading');