Skip to content

Allow wrapping context provider in lazy #26792

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
sophiebits wants to merge 1 commit into from
Closed

Allow wrapping context provider in lazy #26792

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions packages/react-reconciler/src/ReactFiber.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,9 @@ export function resolveLazyComponentTag(Component: Function): WorkTag {
if ($$typeof === REACT_MEMO_TYPE) {
return MemoComponent;
}
if ($$typeof === REACT_PROVIDER_TYPE) {
return ContextProvider;
}
}
return IndeterminateComponent;
}
Expand Down
37 changes: 29 additions & 8 deletions packages/react-reconciler/src/ReactFiberBeginWork.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1843,6 +1843,15 @@ function mountLazyComponent(
);
return child;
}
case ContextProvider: {
child = updateContextProvider(
null,
workInProgress,
resolvedProps,
renderLanes,
);
return child;
}
}
let hint = '';
if (__DEV__) {
Expand Down Expand Up @@ -3487,18 +3496,18 @@ let hasWarnedAboutUsingNoValuePropOnContextProvider = false;
function updateContextProvider(
current: Fiber | null,
workInProgress: Fiber,
nextProps: any,
renderLanes: Lanes,
) {
const providerType: ReactProviderType<any> = workInProgress.type;
const context: ReactContext<any> = providerType._context;

const newProps = workInProgress.pendingProps;
const oldProps = workInProgress.memoizedProps;

const newValue = newProps.value;
const newValue = nextProps.value;

if (__DEV__) {
if (!('value' in newProps)) {
if (!('value' in nextProps)) {
if (!hasWarnedAboutUsingNoValuePropOnContextProvider) {
hasWarnedAboutUsingNoValuePropOnContextProvider = true;
console.error(
Expand All @@ -3509,7 +3518,7 @@ function updateContextProvider(
const providerPropTypes = workInProgress.type.propTypes;

if (providerPropTypes) {
checkPropTypes(providerPropTypes, newProps, 'prop', 'Context.Provider');
checkPropTypes(providerPropTypes, nextProps, 'prop', 'Context.Provider');
Copy link
Collaborator Author

@sophiebits sophiebits May 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we always check these in the reconciler instead of in createElement?

react/packages/react/src/ReactElementValidator.js

Lines 224 to 228 in 16d053d

typeof type === 'object' &&
(type.$$typeof === REACT_FORWARD_REF_TYPE ||
// Note: Memo only checks outer props here.
// Inner props are checked in the reconciler.
type.$$typeof === REACT_MEMO_TYPE)

Copy link
Member

@kassens kassens Oct 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC, there was something subtle about either timing or default prop values? Only vaguely remember this from a comment by @sebmarkbage a while ago.

}
}

Expand All @@ -3526,7 +3535,7 @@ function updateContextProvider(
if (is(oldValue, newValue)) {
// No change. Bailout early if children are the same.
if (
oldProps.children === newProps.children &&
oldProps.children === nextProps.children &&
!hasLegacyContextChanged()
) {
return bailoutOnAlreadyFinishedWork(
Expand All @@ -3543,7 +3552,7 @@ function updateContextProvider(
}
}

const newChildren = newProps.children;
const newChildren = nextProps.children;
reconcileChildren(current, workInProgress, newChildren, renderLanes);
return workInProgress.child;
}
Expand Down Expand Up @@ -4184,8 +4193,20 @@ function beginWork(
return updateMode(current, workInProgress, renderLanes);
case Profiler:
return updateProfiler(current, workInProgress, renderLanes);
case ContextProvider:
return updateContextProvider(current, workInProgress, renderLanes);
case ContextProvider: {
const type = workInProgress.type;
const unresolvedProps = workInProgress.pendingProps;
const resolvedProps =
workInProgress.elementType === type
? unresolvedProps
: resolveDefaultProps(type, unresolvedProps);
return updateContextProvider(
current,
workInProgress,
resolvedProps,
renderLanes,
);
}
case ContextConsumer:
return updateContextConsumer(current, workInProgress, renderLanes);
case MemoComponent: {
Expand Down
37 changes: 37 additions & 0 deletions packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,43 @@ describe('ReactLazy', () => {
expect(root).toMatchRenderedOutput('Hi again');
});

it('renders a lazy context provider', async () => {
const Context = React.createContext('default');
const ConsumerText = () => {
return <Text text={React.useContext(Context)} />;
};
const LazyProvider = lazy(() => fakeImport(Context.Provider));

const root = ReactTestRenderer.create(
<Suspense fallback={<Text text="Loading..." />}>
<LazyProvider value="Hi">
<ConsumerText />
</LazyProvider>
</Suspense>,
{
unstable_isConcurrent: true,
},
);

await waitForAll(['Loading...']);
expect(root).not.toMatchRenderedOutput('Hi');

await act(() => resolveFakeImport(Context.Provider));
assertLog(['Hi']);
expect(root).toMatchRenderedOutput('Hi');

// Should not suspend on update
root.update(
<Suspense fallback={<Text text="Loading..." />}>
<LazyProvider value="Hi again">
<ConsumerText />
</LazyProvider>
</Suspense>,
);
await waitForAll(['Hi again']);
expect(root).toMatchRenderedOutput('Hi again');
});

it('can resolve synchronously without suspending', async () => {
const LazyText = lazy(() => ({
then(cb) {
Expand Down