From 1ebedbec2bec08e07c286ea6c3cff62737a0fd3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Thu, 28 Sep 2023 11:03:19 -0400 Subject: [PATCH] Add Server Context deprecation warning (#27424) As agreed, we're removing Server Context. This was never official documented. We've found that it's not that useful in practice. Often the better options are: - Read things off the url or global scope like params or cookies. - Use the module system for global dependency injection. - Use `React.cache()` to dedupe multiple things instead of computing once and passing down. There are still legit use cases for Server Context but you have to be very careful not to pass any large data, so in generally we recommend against it anyway. Yes, prop drilling is annoying but it's not impossible for the cases this is needed. I would personally always pick it over Server Context anyway. Semantically, Server Context also blocks object deduping due to how it plays out with Server Components that can't be deduped. This is much more important feature. Since it's already in canary along with the rest of RSC, we're adding a warning for a few versions before removing completely to help migration. --------- Co-authored-by: Josh Story --- .../src/__tests__/ReactFlight-test.js | 48 +++++++------- .../src/__tests__/ReactDOMFizzServer-test.js | 19 +++++- .../__tests__/ReactFlightDOMBrowser-test.js | 18 +++++- packages/react/src/ReactServerContext.js | 7 +++ packages/shared/ReactServerContextRegistry.js | 62 ++++++++++++++++--- 5 files changed, 118 insertions(+), 36 deletions(-) diff --git a/packages/react-client/src/__tests__/ReactFlight-test.js b/packages/react-client/src/__tests__/ReactFlight-test.js index c3c3aa420d..2ebbb9f2d5 100644 --- a/packages/react-client/src/__tests__/ReactFlight-test.js +++ b/packages/react-client/src/__tests__/ReactFlight-test.js @@ -98,6 +98,19 @@ describe('ReactFlight', () => { jest.restoreAllMocks(); }); + function createServerContext(globalName, defaultValue, withStack) { + let ctx; + expect(() => { + ctx = React.createServerContext(globalName, defaultValue); + }).toErrorDev( + 'Server Context is deprecated and will soon be removed. ' + + 'It was never documented and we have found it not to be useful ' + + 'enough to warrant the downside it imposes on all apps.', + {withoutStack: !withStack}, + ); + return ctx; + } + function clientReference(value) { return Object.defineProperties( function () { @@ -1063,7 +1076,7 @@ describe('ReactFlight', () => { describe('ServerContext', () => { // @gate enableServerContext it('supports basic createServerContext usage', async () => { - const ServerContext = React.createServerContext( + const ServerContext = createServerContext( 'ServerContext', 'hello from server', ); @@ -1084,10 +1097,7 @@ describe('ReactFlight', () => { // @gate enableServerContext it('propagates ServerContext providers in flight', async () => { - const ServerContext = React.createServerContext( - 'ServerContext', - 'default', - ); + const ServerContext = createServerContext('ServerContext', 'default'); function Foo() { return ( @@ -1115,7 +1125,7 @@ describe('ReactFlight', () => { // @gate enableServerContext it('errors if you try passing JSX through ServerContext value', () => { - const ServerContext = React.createServerContext('ServerContext', { + const ServerContext = createServerContext('ServerContext', { foo: { bar: hi this is default, }, @@ -1149,10 +1159,7 @@ describe('ReactFlight', () => { // @gate enableServerContext it('propagates ServerContext and cleans up the providers in flight', async () => { - const ServerContext = React.createServerContext( - 'ServerContext', - 'default', - ); + const ServerContext = createServerContext('ServerContext', 'default'); function Foo() { return ( @@ -1196,10 +1203,7 @@ describe('ReactFlight', () => { // @gate enableServerContext it('propagates ServerContext providers in flight after suspending', async () => { - const ServerContext = React.createServerContext( - 'ServerContext', - 'default', - ); + const ServerContext = createServerContext('ServerContext', 'default'); function Foo() { return ( @@ -1254,10 +1258,7 @@ describe('ReactFlight', () => { // @gate enableServerContext it('serializes ServerContext to client', async () => { - const ServerContext = React.createServerContext( - 'ServerContext', - 'default', - ); + const ServerContext = createServerContext('ServerContext', 'default'); function ClientBar() { Scheduler.log('ClientBar'); @@ -1294,16 +1295,13 @@ describe('ReactFlight', () => { expect(ReactNoop).toMatchRenderedOutput(hi this is server); expect(() => { - React.createServerContext('ServerContext', 'default'); + createServerContext('ServerContext', 'default'); }).toThrow('ServerContext: ServerContext already defined'); }); // @gate enableServerContext it('takes ServerContext from the client for refetching use cases', async () => { - const ServerContext = React.createServerContext( - 'ServerContext', - 'default', - ); + const ServerContext = createServerContext('ServerContext', 'default'); function Bar() { return {React.useContext(ServerContext)}; } @@ -1323,7 +1321,7 @@ describe('ReactFlight', () => { let ServerContext; function inlineLazyServerContextInitialization() { if (!ServerContext) { - ServerContext = React.createServerContext('ServerContext', 'default'); + ServerContext = createServerContext('ServerContext', 'default'); } return ServerContext; } @@ -1331,7 +1329,7 @@ describe('ReactFlight', () => { let ClientContext; function inlineContextInitialization() { if (!ClientContext) { - ClientContext = React.createServerContext('ServerContext', 'default'); + ClientContext = createServerContext('ServerContext', 'default', true); } return ClientContext; } diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index 44f6f2e64c..9b5e245500 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -3321,12 +3321,19 @@ describe('ReactDOMFizzServer', () => { let ServerContext; function inlineLazyServerContextInitialization() { if (!ServerContext) { - ServerContext = React.createServerContext('ServerContext', 'default'); + expect(() => { + ServerContext = React.createServerContext('ServerContext', 'default'); + }).toErrorDev( + 'Server Context is deprecated and will soon be removed. ' + + 'It was never documented and we have found it not to be useful ' + + 'enough to warrant the downside it imposes on all apps.', + ); } return ServerContext; } function Foo() { + React.useState(); // component stack generation shouldn't reinit inlineLazyServerContextInitialization(); return ( <> @@ -5604,7 +5611,15 @@ describe('ReactDOMFizzServer', () => { it('basic use(context)', async () => { const ContextA = React.createContext('default'); const ContextB = React.createContext('B'); - const ServerContext = React.createServerContext('ServerContext', 'default'); + let ServerContext; + expect(() => { + ServerContext = React.createServerContext('ServerContext', 'default'); + }).toErrorDev( + 'Server Context is deprecated and will soon be removed. ' + + 'It was never documented and we have found it not to be useful ' + + 'enough to warrant the downside it imposes on all apps.', + {withoutStack: true}, + ); function Client() { return use(ContextA) + use(ContextB); } diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js index 9b9697d37e..af474dc148 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js @@ -609,8 +609,22 @@ describe('ReactFlightDOMBrowser', () => { }); it('basic use(context)', async () => { - const ContextA = React.createServerContext('ContextA', ''); - const ContextB = React.createServerContext('ContextB', 'B'); + let ContextA; + let ContextB; + expect(() => { + ContextA = React.createServerContext('ContextA', ''); + ContextB = React.createServerContext('ContextB', 'B'); + }).toErrorDev( + [ + 'Server Context is deprecated and will soon be removed. ' + + 'It was never documented and we have found it not to be useful ' + + 'enough to warrant the downside it imposes on all apps.', + 'Server Context is deprecated and will soon be removed. ' + + 'It was never documented and we have found it not to be useful ' + + 'enough to warrant the downside it imposes on all apps.', + ], + {withoutStack: true}, + ); function ServerComponent() { return use(ContextA) + use(ContextB); diff --git a/packages/react/src/ReactServerContext.js b/packages/react/src/ReactServerContext.js index 6c1feb6cbe..3748a05b12 100644 --- a/packages/react/src/ReactServerContext.js +++ b/packages/react/src/ReactServerContext.js @@ -30,6 +30,13 @@ export function createServerContext( if (!enableServerContext) { throw new Error('Not implemented.'); } + if (__DEV__) { + console.error( + 'Server Context is deprecated and will soon be removed. ' + + 'It was never documented and we have found it not to be useful ' + + 'enough to warrant the downside it imposes on all apps.', + ); + } let wasDefined = true; if (!ContextRegistry[globalName]) { wasDefined = false; diff --git a/packages/shared/ReactServerContextRegistry.js b/packages/shared/ReactServerContextRegistry.js index 8b106d890b..f5bbbae626 100644 --- a/packages/shared/ReactServerContextRegistry.js +++ b/packages/shared/ReactServerContextRegistry.js @@ -9,9 +9,13 @@ import type {ReactServerContext} from 'shared/ReactTypes'; -import {REACT_SERVER_CONTEXT_DEFAULT_VALUE_NOT_LOADED} from 'shared/ReactSymbols'; +import { + REACT_PROVIDER_TYPE, + REACT_SERVER_CONTEXT_TYPE, + REACT_SERVER_CONTEXT_DEFAULT_VALUE_NOT_LOADED, +} from 'shared/ReactSymbols'; + import ReactSharedInternals from 'shared/ReactSharedInternals'; -import {createServerContext} from 'react'; const ContextRegistry = ReactSharedInternals.ContextRegistry; @@ -19,11 +23,55 @@ export function getOrCreateServerContext( globalName: string, ): ReactServerContext { if (!ContextRegistry[globalName]) { - ContextRegistry[globalName] = createServerContext( - globalName, - // $FlowFixMe[incompatible-call] function signature doesn't reflect the symbol value - REACT_SERVER_CONTEXT_DEFAULT_VALUE_NOT_LOADED, - ); + const context: ReactServerContext = { + $$typeof: REACT_SERVER_CONTEXT_TYPE, + + // As a workaround to support multiple concurrent renderers, we categorize + // some renderers as primary and others as secondary. We only expect + // there to be two concurrent renderers at most: React Native (primary) and + // Fabric (secondary); React DOM (primary) and React ART (secondary). + // Secondary renderers store their context values on separate fields. + _currentValue: REACT_SERVER_CONTEXT_DEFAULT_VALUE_NOT_LOADED, + _currentValue2: REACT_SERVER_CONTEXT_DEFAULT_VALUE_NOT_LOADED, + + _defaultValue: REACT_SERVER_CONTEXT_DEFAULT_VALUE_NOT_LOADED, + + // Used to track how many concurrent renderers this context currently + // supports within in a single renderer. Such as parallel server rendering. + _threadCount: 0, + // These are circular + Provider: (null: any), + Consumer: (null: any), + _globalName: globalName, + }; + + context.Provider = { + $$typeof: REACT_PROVIDER_TYPE, + _context: context, + }; + + if (__DEV__) { + let hasWarnedAboutUsingConsumer; + context._currentRenderer = null; + context._currentRenderer2 = null; + Object.defineProperties( + context, + ({ + Consumer: { + get() { + if (!hasWarnedAboutUsingConsumer) { + console.error( + 'Consumer pattern is not supported by ReactServerContext', + ); + hasWarnedAboutUsingConsumer = true; + } + return null; + }, + }, + }: any), + ); + } + ContextRegistry[globalName] = context; } return ContextRegistry[globalName]; }