Skip to content
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

Jump to bottom

Switch <Context> to mean <Context.Provider> #28226

Merged
merged 3 commits into from
Feb 13, 2024
Merged

Switch <Context> to mean <Context.Provider> #28226

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
30ad348
Switch <Context> to mean <Context.Provider>
gaearon Feb 3, 2024
0b14570
Fix DevTools
gaearon Feb 3, 2024
c86765f
Add a feature flag for new behavior
gaearon Feb 11, 2024
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
7 changes: 6 additions & 1 deletion packages/react-client/src/ReactFlightReplyClient.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@ import type {
RejectedThenable,
ReactCustomFormAction,
} from 'shared/ReactTypes';
import {enableRenderableContext} from 'shared/ReactFeatureFlags';

import {
REACT_ELEMENT_TYPE,
REACT_LAZY_TYPE,
REACT_CONTEXT_TYPE,
REACT_PROVIDER_TYPE,
getIteratorFn,
} from 'shared/ReactSymbols';
Expand Down Expand Up @@ -297,7 +299,10 @@ export function processReply(
'React Lazy cannot be passed to Server Functions from the Client.%s',
describeObjectForErrorMessage(parent, key),
);
} else if ((value: any).$$typeof === REACT_PROVIDER_TYPE) {
} else if (
(value: any).$$typeof ===
(enableRenderableContext ? REACT_CONTEXT_TYPE : REACT_PROVIDER_TYPE)
) {
console.error(
'React Context Providers cannot be passed to Server Functions from the Client.%s',
describeObjectForErrorMessage(parent, key),
Expand Down
8 changes: 5 additions & 3 deletions packages/react-debug-tools/src/ReactDebugHooks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import type {
Awaited,
ReactContext,
ReactProviderType,
StartTransitionOptions,
Usable,
} from 'shared/ReactTypes';
Expand Down Expand Up @@ -822,8 +821,11 @@ function setupContexts(contextMap: Map<ReactContext<any>, any>, fiber: Fiber) {
let current: null | Fiber = fiber;
while (current) {
if (current.tag === ContextProvider) {
const providerType: ReactProviderType<any> = current.type;
const context: ReactContext<any> = providerType._context;
let context: ReactContext<any> = current.type;
if ((context: any)._context !== undefined) {
// Support inspection of pre-19+ providers.
context = (context: any)._context;
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note: this isn't feature-flagged because it needs to support both.

if (!contextMap.has(context)) {
// Store the current value that we're going to restore later.
contextMap.set(context, context._currentValue);
Expand Down
2 changes: 2 additions & 0 deletions packages/react-devtools-shared/src/backend/ReactSymbols.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ export const PROFILER_SYMBOL_STRING = 'Symbol(react.profiler)';
export const PROVIDER_NUMBER = 0xeacd;
export const PROVIDER_SYMBOL_STRING = 'Symbol(react.provider)';

export const CONSUMER_SYMBOL_STRING = 'Symbol(react.consumer)';

export const SCOPE_NUMBER = 0xead7;
export const SCOPE_SYMBOL_STRING = 'Symbol(react.scope)';

Expand Down
53 changes: 51 additions & 2 deletions packages/react-devtools-shared/src/backend/renderer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ import {
PROVIDER_SYMBOL_STRING,
CONTEXT_NUMBER,
CONTEXT_SYMBOL_STRING,
CONSUMER_SYMBOL_STRING,
STRICT_MODE_NUMBER,
STRICT_MODE_SYMBOL_STRING,
PROFILER_NUMBER,
Expand Down Expand Up @@ -525,6 +526,15 @@ export function getInternalReactConstants(version: string): {
case CONTEXT_NUMBER:
case CONTEXT_SYMBOL_STRING:
case SERVER_CONTEXT_SYMBOL_STRING:
if (
fiber.type._context === undefined &&
fiber.type.Provider === fiber.type
) {
// In 19+, Context.Provider === Context, so this is a provider.
resolvedContext = fiber.type;
return `${resolvedContext.displayName || 'Context'}.Provider`;
}

// 16.3-16.5 read from "type" because the Consumer is the actual context object.
// 16.6+ should read from "type._context" because Consumer can be different (in DEV).
// NOTE Keep in sync with inspectElementRaw()
Expand All @@ -533,6 +543,10 @@ export function getInternalReactConstants(version: string): {
// NOTE: TraceUpdatesBackendManager depends on the name ending in '.Consumer'
// If you change the name, figure out a more resilient way to detect it.
return `${resolvedContext.displayName || 'Context'}.Consumer`;
case CONSUMER_SYMBOL_STRING:
// 19+
resolvedContext = fiber.type._context;
return `${resolvedContext.displayName || 'Context'}.Consumer`;
case STRICT_MODE_NUMBER:
case STRICT_MODE_SYMBOL_STRING:
return null;
Expand Down Expand Up @@ -3178,8 +3192,14 @@ export function attach(
}
}
} else if (
typeSymbol === CONTEXT_NUMBER ||
typeSymbol === CONTEXT_SYMBOL_STRING
// Detect pre-19 Context Consumers
(typeSymbol === CONTEXT_NUMBER || typeSymbol === CONTEXT_SYMBOL_STRING) &&
!(
// In 19+, CONTEXT_SYMBOL_STRING means a Provider instead.
// It will be handled in a different branch below.
// Eventually, this entire branch can be removed.
(type._context === undefined && type.Provider === type)
)
) {
// 16.3-16.5 read from "type" because the Consumer is the actual context object.
// 16.6+ should read from "type._context" because Consumer can be different (in DEV).
Expand Down Expand Up @@ -3209,6 +3229,35 @@ export function attach(
}
}

current = current.return;
}
} else if (
// Detect 19+ Context Consumers
typeSymbol === CONSUMER_SYMBOL_STRING
) {
// This branch is 19+ only, where Context.Provider === Context.
// NOTE Keep in sync with getDisplayNameForFiber()
const consumerResolvedContext = type._context;

// Global context value.
context = consumerResolvedContext._currentValue || null;

// Look for overridden value.
let current = ((fiber: any): Fiber).return;
while (current !== null) {
const currentType = current.type;
const currentTypeSymbol = getTypeSymbol(currentType);
if (
// In 19+, these are Context Providers
currentTypeSymbol === CONTEXT_SYMBOL_STRING
) {
const providerResolvedContext = currentType;
if (providerResolvedContext === consumerResolvedContext) {
context = current.memoizedProps.value;
break;
}
}

current = current.return;
}
}
Expand Down
133 changes: 26 additions & 107 deletions packages/react-dom/src/__tests__/ReactDOMServerIntegrationNewContext-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,7 @@ function initModules() {
};
}

const {resetModules, itRenders, clientRenderOnBadMarkup} =
ReactDOMServerIntegrationUtils(initModules);
const {resetModules, itRenders} = ReactDOMServerIntegrationUtils(initModules);

describe('ReactDOMServerIntegration', () => {
beforeEach(() => {
Expand Down Expand Up @@ -296,115 +295,35 @@ describe('ReactDOMServerIntegration', () => {
expect(e.querySelector('#language3').textContent).toBe('french');
});

itRenders(
'should warn with an error message when using Context as consumer in DEV',
async render => {
const Theme = React.createContext('dark');
const Language = React.createContext('french');
itRenders('should treat Context as Context.Provider', async render => {
// The `itRenders` helpers don't work with the gate pragma, so we have to do
// this instead.
if (gate(flags => !flags.enableRenderableContext)) {
return;
}

const App = () => (
<div>
<Theme.Provider value="light">
<Language.Provider value="english">
<Theme.Provider value="dark">
<Theme>{theme => <div id="theme1">{theme}</div>}</Theme>
</Theme.Provider>
</Language.Provider>
</Theme.Provider>
</div>
);
// We expect 1 error.
await render(<App />, 1);
},
);

// False positive regression test.
itRenders(
'should not warn when using Consumer from React < 16.6 with newer renderer',
async render => {
const Theme = React.createContext('dark');
const Language = React.createContext('french');
// React 16.5 and earlier didn't have a separate object.
Theme.Consumer = Theme;

const App = () => (
<div>
<Theme.Provider value="light">
<Language.Provider value="english">
<Theme.Provider value="dark">
<Theme>{theme => <div id="theme1">{theme}</div>}</Theme>
</Theme.Provider>
</Language.Provider>
</Theme.Provider>
</div>
);
// We expect 0 errors.
await render(<App />, 0);
},
);

itRenders(
'should warn with an error message when using nested context consumers in DEV',
async render => {
const App = () => {
const Theme = React.createContext('dark');
const Language = React.createContext('french');
const Theme = React.createContext('dark');
const Language = React.createContext('french');

return (
<div>
<Theme.Provider value="light">
<Language.Provider value="english">
<Theme.Provider value="dark">
<Theme.Consumer.Consumer>
{theme => <div id="theme1">{theme}</div>}
</Theme.Consumer.Consumer>
</Theme.Provider>
</Language.Provider>
</Theme.Provider>
</div>
);
};
await render(
<App />,
render === clientRenderOnBadMarkup
? // On hydration mismatch we retry and therefore log the warning again.
2
: 1,
);
},
);
expect(Theme.Provider).toBe(Theme);

itRenders(
'should warn with an error message when using Context.Consumer.Provider DEV',
async render => {
const App = () => {
const Theme = React.createContext('dark');
const Language = React.createContext('french');
const App = () => (
<div>
<Theme value="light">
<Language value="english">
<Theme value="dark">
<Theme.Consumer>
{theme => <div id="theme1">{theme}</div>}
</Theme.Consumer>
</Theme>
</Language>
</Theme>
</div>
);

return (
<div>
<Theme.Provider value="light">
<Language.Provider value="english">
<Theme.Consumer.Provider value="dark">
<Theme.Consumer>
{theme => <div id="theme1">{theme}</div>}
</Theme.Consumer>
</Theme.Consumer.Provider>
</Language.Provider>
</Theme.Provider>
</div>
);
};

await render(
<App />,
render === clientRenderOnBadMarkup
? // On hydration mismatch we retry and therefore log the warning again.
2
: 1,
);
},
);
const e = await render(<App />, 0);
expect(e.textContent).toBe('dark');
});

it('does not pollute parallel node streams', () => {
const LoggedInUser = React.createContext();
Expand Down
24 changes: 9 additions & 15 deletions packages/react-dom/src/__tests__/ReactServerRendering-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1000,22 +1000,15 @@ describe('ReactDOMServer', () => {
]);
});

// @gate enableRenderableContext || !__DEV__
it('should warn if an invalid contextType is defined', () => {
const Context = React.createContext();

class ComponentA extends React.Component {
// It should warn for both Context.Consumer and Context.Provider
static contextType = Context.Consumer;
render() {
return <div />;
}
}
class ComponentB extends React.Component {
static contextType = Context.Provider;
render() {
return <div />;
}
}

expect(() => {
ReactDOMServer.renderToString(<ComponentA />);
Expand All @@ -1028,13 +1021,14 @@ describe('ReactDOMServer', () => {
// Warnings should be deduped by component type
ReactDOMServer.renderToString(<ComponentA />);

expect(() => {
ReactDOMServer.renderToString(<ComponentB />);
}).toErrorDev(
'Warning: ComponentB defines an invalid contextType. ' +
'contextType should point to the Context object returned by React.createContext(). ' +
'Did you accidentally pass the Context.Provider instead?',
);
class ComponentB extends React.Component {
static contextType = Context.Provider;
render() {
return <div />;
}
}
// Does not warn because Context === Context.Provider.
ReactDOMServer.renderToString(<ComponentB />);
});

it('should not warn when class contextType is null', () => {
Expand Down
Loading