Resolve defaultProps for Lazy components

[aweary]

A (still) failing test and naive "fix" for #13960. I want to dig into this more, but hopefully I can get some input on the right approach for this.

[aweary]

The test is failing at componentDidUpdate and getSnapshotBeforeUpdate (this.props), as they are coming back undefined.

[aweary]

The type and tag are overwritten, so my first thought was that the same should be done for pendingProps

[sizebot]

Details of bundled changes.

Comparing: 275e76e...50490ed

scheduler

File	Filesize Diff	Gzip Diff	Prev Size	Current Size	Prev Gzip	Current Gzip	ENV
scheduler.development.js	n/a	n/a	0 B	19.17 KB	0 B	5.74 KB	UMD_DEV
scheduler.production.min.js	n/a	n/a	0 B	3.16 KB	0 B	1.53 KB	UMD_PROD

Generated by 🚫 dangerJS

[aweary]

This solves part of the issue, but the second issue is that we rely on the correct props being resolved in React.createElement. Seems like there will need to be a check for lazy elements so that when a Fiber is created it's created with the resolved default props as well.

The obvious solution is adding a check to createElement for REACT_LAZY_TYPE, but that's a pretty hot path.

[gaearon]

but the second issue is that we rely on the correct props being resolved in React.createElement

When you say "we rely" what do you mean exactly? Rely in what way?

Seems like there will need to be a check for lazy elements so that when a Fiber is created it's created with the resolved default props as well.

I don't understand how this could be possible. If the component is lazy, we don't have its default props at first, do we? We're waiting for it to load.

[gaearon]

I'll continue this in #14108

[aweary]

When you say "we rely" what do you mean exactly? Rely in what way?

Default props are resolved in React.createElement. It's assumed that the element will have the resolved props at that point.

I don't understand how this could be possible. If the component is lazy, we don't have its default props at first, do we? We're waiting for it to load.

The initial load happens inside mountLazyComponent. At that point the default props aren't available, but it suspends and retries and eventually it calls resolveDefaultProps. It passes those resolved props to the corresponding update{Type}Component which is why the props are correct in the constructor and other lifecycle methods called at that point.

react/packages/react-reconciler/src/ReactFiberBeginWork.js

Lines 769 to 774 in d5d10d1

	let Component = readLazyComponentType(elementType);
	// Store the unwrapped component in the type.
	workInProgress.type = Component;
	const resolvedTag = (workInProgress.tag = resolveLazyComponentTag(Component));
	startWorkTimer(workInProgress);
	const resolvedProps = resolveDefaultProps(Component, props);

It was still broken for the initial commit phase, as props get read from the workInProgress. Setting pendingProps to the resolved props fixes that part of the issue.

Even then, it's still broken for updates. In React.createElement the lazy component has been fully resolved, but the default props are not parsed because it checks type.defaultProps and type is not the resolved component. When the workInProgress is created from the element it uses element.props.

My conclusion was that, at some point in the update path, we'd need to check the type and resolve the default props. I was trying to find the right place for this so that non-lazy components didn't incur the cost of calling resolveDefaultProps as well, but it wasn't clear where that would be for updates.

I was thinking it would be a similar solution to how we handle props with fragments

react/packages/react-reconciler/src/ReactChildFiber.js

Lines 1128 to 1130 in d5d10d1

	element.type === REACT_FRAGMENT_TYPE
	? element.props.children
	: element.props,

[gaearon]

but the default props are not parsed because it checks type.defaultProps and type is not the resolved component

Would fiber.elementType work then?

[gaearon]

(Feel free to push to my branch in #14108)

[aweary]

Would fiber.elementType work then?

We could check fiber.elementType before calling createWorkInProgress and then call resolveDefaultProps with pendingProps if it's a lazy component, but I was concerned about adding a property check there since it's a hot path for all element types.