From 7e5dc36727d0bde081c2f5f58525f52a3c5a1002 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 25 Jun 2024 16:41:33 +0200 Subject: [PATCH 1/8] Skip unnecessary checks when enableOwnerStacks is enabled This lets us delete more of this code once it lands fully. In particular the hard coded client reference is no longer needed. --- packages/react/src/jsx/ReactJSXElement.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/packages/react/src/jsx/ReactJSXElement.js b/packages/react/src/jsx/ReactJSXElement.js index 0f8b9f397df5b..b14c186d78c54 100644 --- a/packages/react/src/jsx/ReactJSXElement.js +++ b/packages/react/src/jsx/ReactJSXElement.js @@ -1103,6 +1103,17 @@ export function cloneElement(element, config, children) { */ function validateChildKeys(node, parentType) { if (__DEV__) { + if (enableOwnerStacks) { + // When owner stacks is enabled no warnings happens. All we do is + // mark elements as being in a valid static child position so they + // don't need keys. + if (isValidElement(node)) { + if (node._store) { + node._store.validated = 1; + } + } + return; + } if (typeof node !== 'object' || !node) { return; } From 26088ff12f8380e1465adce9b9c22a3bc592012f Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 25 Jun 2024 17:15:52 +0200 Subject: [PATCH 2/8] Fake fragment fibers should have their debug info assigned --- packages/react-reconciler/src/ReactChildFiber.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/react-reconciler/src/ReactChildFiber.js b/packages/react-reconciler/src/ReactChildFiber.js index db8f0e6233644..10edf8537a53f 100644 --- a/packages/react-reconciler/src/ReactChildFiber.js +++ b/packages/react-reconciler/src/ReactChildFiber.js @@ -220,6 +220,9 @@ function validateFragmentProps( // For unkeyed root fragments there's no Fiber. We create a fake one just for // error stack handling. fiber = createFiberFromElement(element, returnFiber.mode, 0); + if (__DEV__) { + fiber._debugInfo = currentDebugInfo; + } fiber.return = returnFiber; } runWithFiberInDEV( @@ -242,6 +245,9 @@ function validateFragmentProps( // For unkeyed root fragments there's no Fiber. We create a fake one just for // error stack handling. fiber = createFiberFromElement(element, returnFiber.mode, 0); + if (__DEV__) { + fiber._debugInfo = currentDebugInfo; + } fiber.return = returnFiber; } runWithFiberInDEV(fiber, () => { From 6388c2c3064d13e1860490be03f77b66ecee1cd3 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 25 Jun 2024 17:16:17 +0200 Subject: [PATCH 3/8] Disable the type warning for JSX We'll handle this in the reconciler instead with the proper stack traces. --- packages/react/src/jsx/ReactJSXElement.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/react/src/jsx/ReactJSXElement.js b/packages/react/src/jsx/ReactJSXElement.js index b14c186d78c54..1652b1e2edecc 100644 --- a/packages/react/src/jsx/ReactJSXElement.js +++ b/packages/react/src/jsx/ReactJSXElement.js @@ -559,9 +559,14 @@ function jsxDEVImpl( debugTask, ) { if (__DEV__) { - if (!isValidElementType(type)) { + if (!enableOwnerStacks && !isValidElementType(type)) { // This is an invalid element type. // + // We warn here so that we can get better stack traces but with enableOwnerStacks + // enabled we don't need this because we get good stacks if we error in the + // renderer anyway. The renderer is the only one that knows what types are valid + // for this particular renderer so we let it error there instead. + // // We warn in this case but don't throw. We expect the element creation to // succeed and there will likely be errors in render. let info = ''; @@ -604,6 +609,9 @@ function jsxDEVImpl( // errors. We don't want exception behavior to differ between dev and // prod. (Rendering will throw with a helpful message and as soon as the // type is fixed, the key warnings will appear.) + // When enableOwnerStacks is on, we no longer need the type here so this + // comment is no longer true. Which is why we can run this even for invalid + // types. const children = config.children; if (children !== undefined) { if (isStaticChildren) { From ca11a1bf64b38155e380e18f4548af6d98c69e71 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 25 Jun 2024 17:29:29 +0200 Subject: [PATCH 4/8] Create Throw Fiber for invalid types This ensures that we transfer the stack/owner from the element to the new Fiber. --- packages/react-reconciler/src/ReactFiber.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/react-reconciler/src/ReactFiber.js b/packages/react-reconciler/src/ReactFiber.js index da67714d0906f..2911b1bf5c549 100644 --- a/packages/react-reconciler/src/ReactFiber.js +++ b/packages/react-reconciler/src/ReactFiber.js @@ -485,6 +485,7 @@ export function createHostRootFiber( return createFiber(HostRoot, null, null, mode); } +// TODO: Get rid of this helper. Only createFiberFromElement should exist. export function createFiberFromTypeAndProps( type: any, // React$ElementType key: null | string, @@ -650,11 +651,18 @@ export function createFiberFromTypeAndProps( typeString = type === null ? 'null' : typeof type; } - throw new Error( + // The type is invalid but it's conceptually a child that errored and not the + // current component itself so we create a virtual child that throws in its + // begin phase. This is the same thing we do in ReactChildFiber if we throw + // but we do it here so that we can assign the debug owner and stack from the + // element itself. That way the error stack will point to the JSX callsite. + fiberTag = Throw; + pendingProps = new Error( 'Element type is invalid: expected a string (for built-in ' + 'components) or a class/function (for composite components) ' + `but got: ${typeString}.${info}`, ); + resolvedType = null; } } } From a6694b3fd1a93a35e35421b3d7a1eeeb794e06a6 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 25 Jun 2024 17:59:59 +0200 Subject: [PATCH 5/8] Fix tests Covered in the ReactComponent-test --- .../src/__tests__/ReactComponent-test.js | 80 +++++++++++++------ .../ReactDOMServerIntegrationElements-test.js | 32 +++++--- ...eactLegacyErrorBoundaries-test.internal.js | 57 +++++++------ ...rorBoundaryReconciliation-test.internal.js | 23 +++--- ...tIncrementalErrorHandling-test.internal.js | 42 ++++++---- .../ReactElementValidator-test.internal.js | 14 ++-- .../ReactJSXElementValidator-test.js | 29 ------- 7 files changed, 159 insertions(+), 118 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactComponent-test.js b/packages/react-dom/src/__tests__/ReactComponent-test.js index f0d58280cd8c1..81e1ac173e638 100644 --- a/packages/react-dom/src/__tests__/ReactComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactComponent-test.js @@ -14,6 +14,7 @@ let ReactDOM; let ReactDOMClient; let ReactDOMServer; let act; +let assertConsoleErrorDev; describe('ReactComponent', () => { beforeEach(() => { @@ -24,6 +25,8 @@ describe('ReactComponent', () => { ReactDOMClient = require('react-dom/client'); ReactDOMServer = require('react-dom/server'); act = require('internal-test-utils').act; + assertConsoleErrorDev = + require('internal-test-utils').assertConsoleErrorDev; }); // @gate !disableLegacyMode @@ -511,19 +514,25 @@ describe('ReactComponent', () => { }); it('throws usefully when rendering badly-typed elements', async () => { + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + const X = undefined; - let container = document.createElement('div'); - let root = ReactDOMClient.createRoot(container); - await expect( - expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( - 'React.jsx: type is invalid -- expected a string (for built-in components) ' + - 'or a class/function (for composite components) but got: undefined.', - ), - ).rejects.toThrowError( + const XElement = ; + if (gate(flags => !flags.enableOwnerStacks)) { + assertConsoleErrorDev( + [ + 'React.jsx: type is invalid -- expected a string (for built-in components) ' + + 'or a class/function (for composite components) but got: undefined.', + ], + {withoutStack: true}, + ); + } + await expect(async () => { + await act(() => { + root.render(XElement); + }); + }).rejects.toThrowError( 'Element type is invalid: expected a string (for built-in components) ' + 'or a class/function (for composite components) but got: undefined.' + (__DEV__ @@ -533,21 +542,44 @@ describe('ReactComponent', () => { ); const Y = null; - container = document.createElement('div'); - root = ReactDOMClient.createRoot(container); - await expect( - expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( - 'React.jsx: type is invalid -- expected a string (for built-in components) ' + - 'or a class/function (for composite components) but got: null.', - ), - ).rejects.toThrowError( + const YElement = ; + if (gate(flags => !flags.enableOwnerStacks)) { + assertConsoleErrorDev( + [ + 'React.jsx: type is invalid -- expected a string (for built-in components) ' + + 'or a class/function (for composite components) but got: null.', + ], + {withoutStack: true}, + ); + } + await expect(async () => { + await act(() => { + root.render(YElement); + }); + }).rejects.toThrowError( 'Element type is invalid: expected a string (for built-in components) ' + 'or a class/function (for composite components) but got: null.', ); + + const Z = true; + const ZElement = ; + if (gate(flags => !flags.enableOwnerStacks)) { + assertConsoleErrorDev( + [ + 'React.jsx: type is invalid -- expected a string (for built-in components) ' + + 'or a class/function (for composite components) but got: boolean.', + ], + {withoutStack: true}, + ); + } + await expect(async () => { + await act(() => { + root.render(ZElement); + }); + }).rejects.toThrowError( + 'Element type is invalid: expected a string (for built-in components) ' + + 'or a class/function (for composite components) but got: boolean.', + ); }); it('includes owner name in the error about badly-typed elements', async () => { diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationElements-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationElements-test.js index 2912a4f401dec..0fcc314d39a05 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationElements-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationElements-test.js @@ -987,11 +987,13 @@ describe('ReactDOMServerIntegration', () => { expect(() => { EmptyComponent = ; }).toErrorDev( - 'React.jsx: type is invalid -- expected a string ' + - '(for built-in components) or a class/function (for composite ' + - 'components) but got: object. You likely forgot to export your ' + - "component from the file it's defined in, or you might have mixed up " + - 'default and named imports.', + gate(flags => flags.enableOwnerStacks) + ? [] + : 'React.jsx: type is invalid -- expected a string ' + + '(for built-in components) or a class/function (for composite ' + + 'components) but got: object. You likely forgot to export your ' + + "component from the file it's defined in, or you might have mixed up " + + 'default and named imports.', {withoutStack: true}, ); await render(EmptyComponent); @@ -1011,9 +1013,11 @@ describe('ReactDOMServerIntegration', () => { expect(() => { NullComponent = ; }).toErrorDev( - 'React.jsx: type is invalid -- expected a string ' + - '(for built-in components) or a class/function (for composite ' + - 'components) but got: null.', + gate(flags => flags.enableOwnerStacks) + ? [] + : 'React.jsx: type is invalid -- expected a string ' + + '(for built-in components) or a class/function (for composite ' + + 'components) but got: null.', {withoutStack: true}, ); await render(NullComponent); @@ -1029,11 +1033,13 @@ describe('ReactDOMServerIntegration', () => { expect(() => { UndefinedComponent = ; }).toErrorDev( - 'React.jsx: type is invalid -- expected a string ' + - '(for built-in components) or a class/function (for composite ' + - 'components) but got: undefined. You likely forgot to export your ' + - "component from the file it's defined in, or you might have mixed up " + - 'default and named imports.', + gate(flags => flags.enableOwnerStacks) + ? [] + : 'React.jsx: type is invalid -- expected a string ' + + '(for built-in components) or a class/function (for composite ' + + 'components) but got: undefined. You likely forgot to export your ' + + "component from the file it's defined in, or you might have mixed up " + + 'default and named imports.', {withoutStack: true}, ); diff --git a/packages/react-dom/src/__tests__/ReactLegacyErrorBoundaries-test.internal.js b/packages/react-dom/src/__tests__/ReactLegacyErrorBoundaries-test.internal.js index 2809460c5b06b..f45fae7bbd7e1 100644 --- a/packages/react-dom/src/__tests__/ReactLegacyErrorBoundaries-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactLegacyErrorBoundaries-test.internal.js @@ -13,6 +13,7 @@ let PropTypes; let React; let ReactDOM; let act; +let assertConsoleErrorDev; // TODO: Refactor this test once componentDidCatch setState is deprecated. describe('ReactLegacyErrorBoundaries', () => { @@ -42,6 +43,8 @@ describe('ReactLegacyErrorBoundaries', () => { ReactDOM = require('react-dom'); React = require('react'); act = require('internal-test-utils').act; + assertConsoleErrorDev = + require('internal-test-utils').assertConsoleErrorDev; log = []; @@ -2099,32 +2102,38 @@ describe('ReactLegacyErrorBoundaries', () => { const Y = undefined; await expect(async () => { - await expect(async () => { - const container = document.createElement('div'); - await act(() => { - ReactDOM.render(, container); - }); - }).rejects.toThrow('got: null'); - }).toErrorDev( - 'React.jsx: type is invalid -- expected a string ' + - '(for built-in components) or a class/function ' + - '(for composite components) but got: null.', - {withoutStack: 1}, - ); + const container = document.createElement('div'); + await act(() => { + ReactDOM.render(, container); + }); + }).rejects.toThrow('got: null'); + if (gate(flags => !flags.enableOwnerStacks)) { + assertConsoleErrorDev( + [ + 'React.jsx: type is invalid -- expected a string ' + + '(for built-in components) or a class/function ' + + '(for composite components) but got: null.', + ], + {withoutStack: true}, + ); + } await expect(async () => { - await expect(async () => { - const container = document.createElement('div'); - await act(() => { - ReactDOM.render(, container); - }); - }).rejects.toThrow('got: undefined'); - }).toErrorDev( - 'React.jsx: type is invalid -- expected a string ' + - '(for built-in components) or a class/function ' + - '(for composite components) but got: undefined.', - {withoutStack: 1}, - ); + const container = document.createElement('div'); + await act(() => { + ReactDOM.render(, container); + }); + }).rejects.toThrow('got: undefined'); + if (gate(flags => !flags.enableOwnerStacks)) { + assertConsoleErrorDev( + [ + 'React.jsx: type is invalid -- expected a string ' + + '(for built-in components) or a class/function ' + + '(for composite components) but got: undefined.', + ], + {withoutStack: true}, + ); + } }); // @gate !disableLegacyMode diff --git a/packages/react-reconciler/src/__tests__/ErrorBoundaryReconciliation-test.internal.js b/packages/react-reconciler/src/__tests__/ErrorBoundaryReconciliation-test.internal.js index b577bd1bc074e..43f58ffc4bfd5 100644 --- a/packages/react-reconciler/src/__tests__/ErrorBoundaryReconciliation-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ErrorBoundaryReconciliation-test.internal.js @@ -6,6 +6,7 @@ describe('ErrorBoundaryReconciliation', () => { let ReactTestRenderer; let span; let act; + let assertConsoleErrorDev; beforeEach(() => { jest.resetModules(); @@ -13,6 +14,8 @@ describe('ErrorBoundaryReconciliation', () => { ReactTestRenderer = require('react-test-renderer'); React = require('react'); act = require('internal-test-utils').act; + assertConsoleErrorDev = + require('internal-test-utils').assertConsoleErrorDev; DidCatchErrorBoundary = class extends React.Component { state = {error: null}; componentDidCatch(error) { @@ -58,15 +61,17 @@ describe('ErrorBoundaryReconciliation', () => { ); }); expect(renderer).toMatchRenderedOutput(); - await expect(async () => { - await act(() => { - renderer.update( - - - , - ); - }); - }).toErrorDev(['invalid', 'invalid']); + await act(() => { + renderer.update( + + + , + ); + }); + if (gate(flags => !flags.enableOwnerStacks)) { + assertConsoleErrorDev(['invalid', 'invalid']); + } + const Fallback = fallbackTagName; expect(renderer).toMatchRenderedOutput(); } diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js index e800dd74e10a3..0cee8150cb15f 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js @@ -19,6 +19,7 @@ let assertLog; let waitForAll; let waitFor; let waitForThrow; +let assertConsoleErrorDev; describe('ReactIncrementalErrorHandling', () => { beforeEach(() => { @@ -28,6 +29,8 @@ describe('ReactIncrementalErrorHandling', () => { ReactNoop = require('react-noop-renderer'); Scheduler = require('scheduler'); act = require('internal-test-utils').act; + assertConsoleErrorDev = + require('internal-test-utils').assertConsoleErrorDev; const InternalTestUtils = require('internal-test-utils'); assertLog = InternalTestUtils.assertLog; @@ -1237,11 +1240,15 @@ describe('ReactIncrementalErrorHandling', () => { , ); - await expect(async () => await waitForAll([])).toErrorDev([ - 'React.jsx: type is invalid -- expected a string', - // React retries once on error - 'React.jsx: type is invalid -- expected a string', - ]); + await waitForAll([]); + if (gate(flags => !flags.enableOwnerStacks)) { + assertConsoleErrorDev([ + 'React.jsx: type is invalid -- expected a string', + // React retries once on error + 'React.jsx: type is invalid -- expected a string', + ]); + } + expect(ReactNoop).toMatchRenderedOutput( { , ); - await expect(async () => await waitForAll([])).toErrorDev([ - 'React.jsx: type is invalid -- expected a string', - // React retries once on error - 'React.jsx: type is invalid -- expected a string', - ]); + await waitForAll([]); + if (gate(flags => !flags.enableOwnerStacks)) { + assertConsoleErrorDev([ + 'React.jsx: type is invalid -- expected a string', + // React retries once on error + 'React.jsx: type is invalid -- expected a string', + ]); + } expect(ReactNoop).toMatchRenderedOutput( { it('recovers from uncaught reconciler errors', async () => { const InvalidType = undefined; - expect(() => ReactNoop.render()).toErrorDev( - 'React.jsx: type is invalid -- expected a string', - {withoutStack: true}, - ); + ReactNoop.render(); + if (gate(flags => !flags.enableOwnerStacks)) { + assertConsoleErrorDev( + ['React.jsx: type is invalid -- expected a string'], + {withoutStack: true}, + ); + } + await waitForThrow( 'Element type is invalid: expected a string (for built-in components) or ' + 'a class/function (for composite components) but got: undefined.' + diff --git a/packages/react/src/__tests__/ReactElementValidator-test.internal.js b/packages/react/src/__tests__/ReactElementValidator-test.internal.js index 00c92eabe703f..4383a6472d350 100644 --- a/packages/react/src/__tests__/ReactElementValidator-test.internal.js +++ b/packages/react/src/__tests__/ReactElementValidator-test.internal.js @@ -515,11 +515,15 @@ describe('ReactElementValidator', () => { expect(() => { void ({[
]}); }).toErrorDev( - 'React.jsx: type is invalid -- expected a string ' + - '(for built-in components) or a class/function (for composite ' + - 'components) but got: undefined. You likely forgot to export your ' + - "component from the file it's defined in, or you might have mixed up " + - 'default and named imports.', + gate(flags => flags.enableOwnerStacks) + ? [] + : [ + 'React.jsx: type is invalid -- expected a string ' + + '(for built-in components) or a class/function (for composite ' + + 'components) but got: undefined. You likely forgot to export your ' + + "component from the file it's defined in, or you might have mixed up " + + 'default and named imports.', + ], {withoutStack: true}, ); }); diff --git a/packages/react/src/__tests__/ReactJSXElementValidator-test.js b/packages/react/src/__tests__/ReactJSXElementValidator-test.js index 4fc666aa4e7fd..f827a52bc59ca 100644 --- a/packages/react/src/__tests__/ReactJSXElementValidator-test.js +++ b/packages/react/src/__tests__/ReactJSXElementValidator-test.js @@ -215,35 +215,6 @@ describe('ReactJSXElementValidator', () => { ); }); - it('gives a helpful error when passing null, undefined, or boolean', () => { - const Undefined = undefined; - const Null = null; - const True = true; - const Div = 'div'; - expect(() => void ()).toErrorDev( - 'React.jsx: type is invalid -- expected a string ' + - '(for built-in components) or a class/function (for composite ' + - 'components) but got: undefined. You likely forgot to export your ' + - "component from the file it's defined in, or you might have mixed up " + - 'default and named imports.', - {withoutStack: true}, - ); - expect(() => void ()).toErrorDev( - 'React.jsx: type is invalid -- expected a string ' + - '(for built-in components) or a class/function (for composite ' + - 'components) but got: null.', - {withoutStack: true}, - ); - expect(() => void ()).toErrorDev( - 'React.jsx: type is invalid -- expected a string ' + - '(for built-in components) or a class/function (for composite ' + - 'components) but got: boolean.', - {withoutStack: true}, - ); - // No error expected - void (
); - }); - it('warns for fragments with illegal attributes', async () => { class Foo extends React.Component { render() { From eb947b5aab83df5582860c20abf15ecf087495d7 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 25 Jun 2024 18:47:55 +0200 Subject: [PATCH 6/8] Serialize all unknown types of React Elements We let the client decide if it's valid or not and provide the error message if not. --- .../react-server/src/ReactFlightServer.js | 84 +++++++------------ 1 file changed, 31 insertions(+), 53 deletions(-) diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 02847f204bc3e..b6936e69dbe3f 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -1501,19 +1501,11 @@ function renderElement( jsxChildrenParents.set(props.children, type); } } - if (typeof type === 'function') { - if (isClientReference(type) || isOpaqueTemporaryReference(type)) { - // This is a reference to a Client Component. - return renderClientElement( - task, - type, - key, - props, - owner, - stack, - validated, - ); - } + if ( + typeof type === 'function' && + !isClientReference(type) && + !isOpaqueTemporaryReference(type) + ) { // This is a Server Component. return renderFunctionComponent( request, @@ -1525,43 +1517,27 @@ function renderElement( stack, validated, ); - } else if (typeof type === 'string') { - // This is a host element. E.g. HTML. - return renderClientElement(task, type, key, props, owner, stack, validated); - } else if (typeof type === 'symbol') { - if (type === REACT_FRAGMENT_TYPE && key === null) { - // For key-less fragments, we add a small optimization to avoid serializing - // it as a wrapper. - const prevImplicitSlot = task.implicitSlot; - if (task.keyPath === null) { - task.implicitSlot = true; - } - const json = renderModelDestructive( - request, - task, - emptyRoot, - '', - props.children, - ); - task.implicitSlot = prevImplicitSlot; - return json; - } - // This might be a built-in React component. We'll let the client decide. - // Any built-in works as long as its props are serializable. - return renderClientElement(task, type, key, props, owner, stack, validated); - } else if (type != null && typeof type === 'object') { - if (isClientReference(type)) { - // This is a reference to a Client Component. - return renderClientElement( - task, - type, - key, - props, - owner, - stack, - validated, - ); - } + } else if (type === REACT_FRAGMENT_TYPE && key === null) { + // For key-less fragments, we add a small optimization to avoid serializing + // it as a wrapper. + const prevImplicitSlot = task.implicitSlot; + if (task.keyPath === null) { + task.implicitSlot = true; + } + const json = renderModelDestructive( + request, + task, + emptyRoot, + '', + props.children, + ); + task.implicitSlot = prevImplicitSlot; + return json; + } else if ( + type != null && + typeof type === 'object' && + !isClientReference(type) + ) { switch (type.$$typeof) { case REACT_LAZY_TYPE: { let wrappedType; @@ -1617,9 +1593,11 @@ function renderElement( } } } - throw new Error( - `Unsupported Server Component type: ${describeValueForErrorMessage(type)}`, - ); + // For anything else, try it on the client instead. + // We don't know if the client will support it or not. This might error on the + // client or error during serialization but the stack will point back to the + // server. + return renderClientElement(task, type, key, props, owner, stack, validated); } function pingTask(request: Request, task: Task): void { From fdfec40a9837162e80248c8eb6f7adf593b33181 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 26 Jun 2024 13:17:13 -0400 Subject: [PATCH 7/8] Disable key warning when element used as type This gets serialized as a tuple in the Flight protocol so we think it needs a key but it doesn't. It'll error later on the client. Fix test asserting lazy because it's not actually executing the rendering pass on the client since the transport is not live in Noop. Technically this doesn't error if the Shared Component renders something that can be a type since we actually resolve this component as if it is just some value. --- .../src/__tests__/ReactFlight-test.js | 22 +++++++++++++------ .../react-server/src/ReactFlightServer.js | 9 +++++++- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/packages/react-client/src/__tests__/ReactFlight-test.js b/packages/react-client/src/__tests__/ReactFlight-test.js index 8a652dab4aaa8..2e3857a90deca 100644 --- a/packages/react-client/src/__tests__/ReactFlight-test.js +++ b/packages/react-client/src/__tests__/ReactFlight-test.js @@ -692,14 +692,22 @@ describe('ReactFlight', () => { const transport = ReactNoopFlightServer.render(); - await act(async () => { - const rootModel = await ReactNoopFlightClient.read(transport); - ReactNoop.render(rootModel); - }); - expect(ReactNoop).toMatchRenderedOutput('Loading...'); - spyOnDevAndProd(console, 'error').mockImplementation(() => {}); await load(); - expect(console.error).toHaveBeenCalledTimes(1); + + await expect(async () => { + await act(async () => { + const rootModel = await ReactNoopFlightClient.read(transport); + ReactNoop.render(rootModel); + }); + }).rejects.toThrow( + __DEV__ + ? 'Element type is invalid: expected a string (for built-in components) or a class/function ' + + '(for composite components) but got:
. ' + + 'Did you accidentally export a JSX literal instead of a component?' + : 'Element type is invalid: expected a string (for built-in components) or a class/function ' + + '(for composite components) but got: object.', + ); + expect(ReactNoop).toMatchRenderedOutput(null); }); it('can render a lazy element', async () => { diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index b6936e69dbe3f..9987c689f2aa8 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -110,7 +110,6 @@ import { } from 'shared/ReactSymbols'; import { - describeValueForErrorMessage, describeObjectForErrorMessage, isSimpleObject, jsxPropsParents, @@ -1591,6 +1590,14 @@ function renderElement( validated, ); } + case REACT_ELEMENT_TYPE: { + // This is invalid but we'll let the client determine that it is. + if (__DEV__) { + // Disable the key warning that would happen otherwise because this + // element gets serialized inside an array. We'll error later anyway. + type._store.validated = 1; + } + } } } // For anything else, try it on the client instead. From 213d584df967b09dcc239f9ecb49b39097373277 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 26 Jun 2024 13:37:42 -0400 Subject: [PATCH 8/8] Remove spy It breaks the other tests. --- packages/react-dom/src/__tests__/ReactComponent-test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactComponent-test.js b/packages/react-dom/src/__tests__/ReactComponent-test.js index 81e1ac173e638..9b2f443a7c34d 100644 --- a/packages/react-dom/src/__tests__/ReactComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactComponent-test.js @@ -134,8 +134,6 @@ describe('ReactComponent', () => { // @gate !disableStringRefs it('string refs do not detach and reattach on every render', async () => { - spyOnDev(console, 'error').mockImplementation(() => {}); - let refVal; class Child extends React.Component { componentDidUpdate() { @@ -174,6 +172,8 @@ describe('ReactComponent', () => { root.render(); }); + assertConsoleErrorDev(['contains the string ref']); + expect(refVal).toBe(undefined); await act(() => { root.render();