From 922dd7ba50f69f1f969c5809eb57104768716c89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Thu, 2 Feb 2023 15:30:59 -0500 Subject: [PATCH] Revert the outer module object to an object (#26093) This is because Webpack has a `typeof ... === 'object'` before its esm compat test. This is unfortunate because it means we can't have a nice error in CJS when someone does this: ``` const fn = require('client-fn'); fn(); ``` I also fixed some checks in the validator that read off the client ref. It shouldn't do those checks against a client ref, since those now throw. --- .../src/ReactFlightWebpackNodeRegister.js | 53 +++-------- .../src/__tests__/ReactFlightDOM-test.js | 88 +++++++++++++++++++ packages/react/src/ReactElementValidator.js | 13 ++- .../react/src/jsx/ReactJSXElementValidator.js | 13 ++- 4 files changed, 122 insertions(+), 45 deletions(-) diff --git a/packages/react-server-dom-webpack/src/ReactFlightWebpackNodeRegister.js b/packages/react-server-dom-webpack/src/ReactFlightWebpackNodeRegister.js index d9512a69a5a07..1f3202bdc153b 100644 --- a/packages/react-server-dom-webpack/src/ReactFlightWebpackNodeRegister.js +++ b/packages/react-server-dom-webpack/src/ReactFlightWebpackNodeRegister.js @@ -33,8 +33,6 @@ module.exports = function register() { // reference. case 'defaultProps': return undefined; - case 'getDefaultProps': - return undefined; // Avoid this attempting to be serialized. case 'toJSON': return undefined; @@ -91,8 +89,6 @@ module.exports = function register() { // reference. case 'defaultProps': return undefined; - case 'getDefaultProps': - return undefined; // Avoid this attempting to be serialized. case 'toJSON': return undefined; @@ -132,24 +128,13 @@ module.exports = function register() { // we should resolve that with a client reference that unwraps the Promise on // the client. - const innerModuleId = target.filepath; - const clientReference: Function = Object.defineProperties( - (function () { - throw new Error( - `Attempted to call the module exports of ${innerModuleId} from the server` + - `but it's on the client. It's not possible to invoke a client function from ` + - `the server, it can only be rendered as a Component or passed to props of a` + - `Client Component.`, - ); - }: any), - { - // Represents the whole object instead of a particular import. - name: {value: '*'}, - $$typeof: {value: CLIENT_REFERENCE}, - filepath: {value: target.filepath}, - async: {value: true}, - }, - ); + const clientReference = Object.defineProperties(({}: any), { + // Represents the whole Module object instead of a particular import. + name: {value: '*'}, + $$typeof: {value: CLIENT_REFERENCE}, + filepath: {value: target.filepath}, + async: {value: true}, + }); const proxy = new Proxy(clientReference, proxyHandlers); // Treat this as a resolved Promise for React's use() @@ -221,23 +206,13 @@ module.exports = function register() { // $FlowFixMe[prop-missing] found when upgrading Flow Module._extensions['.client.js'] = function (module, path) { const moduleId: string = (url.pathToFileURL(path).href: any); - const clientReference: Function = Object.defineProperties( - (function () { - throw new Error( - `Attempted to call the module exports of ${moduleId} from the server` + - `but it's on the client. It's not possible to invoke a client function from ` + - `the server, it can only be rendered as a Component or passed to props of a` + - `Client Component.`, - ); - }: any), - { - // Represents the whole object instead of a particular import. - name: {value: '*'}, - $$typeof: {value: CLIENT_REFERENCE}, - filepath: {value: moduleId}, - async: {value: false}, - }, - ); + const clientReference = Object.defineProperties(({}: any), { + // Represents the whole Module object instead of a particular import. + name: {value: '*'}, + $$typeof: {value: CLIENT_REFERENCE}, + filepath: {value: moduleId}, + async: {value: false}, + }); // $FlowFixMe[incompatible-call] found when upgrading Flow module.exports = new Proxy(clientReference, proxyHandlers); }; diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js index 36fcf31970477..36c3139c1af3d 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js @@ -246,6 +246,94 @@ describe('ReactFlightDOM', () => { expect(container.innerHTML).toBe('

@div

'); }); + // @gate enableUseHook + it('should be able to esm compat test module references', async () => { + const ESMCompatModule = { + __esModule: true, + default: function ({greeting}) { + return greeting + ' World'; + }, + hi: 'Hello', + }; + + function Print({response}) { + return

{use(response)}

; + } + + function App({response}) { + return ( + Loading...}> + + + ); + } + + function interopWebpack(obj) { + // Basically what Webpack's ESM interop feature testing does. + if (typeof obj === 'object' && obj.__esModule) { + return obj; + } + return Object.assign({default: obj}, obj); + } + + const {default: Component, hi} = interopWebpack( + clientExports(ESMCompatModule), + ); + + const {writable, readable} = getTestStream(); + const {pipe} = ReactServerDOMWriter.renderToPipeableStream( + , + webpackMap, + ); + pipe(writable); + const response = ReactServerDOMReader.createFromReadableStream(readable); + + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await act(async () => { + root.render(); + }); + expect(container.innerHTML).toBe('

Hello World

'); + }); + + // @gate enableUseHook + it('should be able to render a named component export', async () => { + const Module = { + Component: function ({greeting}) { + return greeting + ' World'; + }, + }; + + function Print({response}) { + return

{use(response)}

; + } + + function App({response}) { + return ( + Loading...}> + + + ); + } + + const {Component} = clientExports(Module); + + const {writable, readable} = getTestStream(); + const {pipe} = ReactServerDOMWriter.renderToPipeableStream( + , + webpackMap, + ); + pipe(writable); + const response = ReactServerDOMReader.createFromReadableStream(readable); + + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await act(async () => { + root.render(); + }); + expect(container.innerHTML).toBe('

Hello World

'); + }); + // @gate enableUseHook it('should unwrap async module references', async () => { const AsyncModule = Promise.resolve(function AsyncModule({text}) { diff --git a/packages/react/src/ReactElementValidator.js b/packages/react/src/ReactElementValidator.js index 833337910f611..0451025b6c7f1 100644 --- a/packages/react/src/ReactElementValidator.js +++ b/packages/react/src/ReactElementValidator.js @@ -35,6 +35,8 @@ import {setExtraStackFrame} from './ReactDebugCurrentFrame'; import {describeUnknownElementTypeFrameInDEV} from 'shared/ReactComponentStackFrame'; import hasOwnProperty from 'shared/hasOwnProperty'; +const REACT_CLIENT_REFERENCE = Symbol.for('react.client.reference'); + function setCurrentlyValidatingElement(element) { if (__DEV__) { if (element) { @@ -165,10 +167,12 @@ function validateExplicitKey(element, parentType) { * @param {*} parentType node's parent's type. */ function validateChildKeys(node, parentType) { - if (typeof node !== 'object') { + if (typeof node !== 'object' || !node) { return; } - if (isArray(node)) { + if (node.$$typeof === REACT_CLIENT_REFERENCE) { + // This is a reference to a client component so it's unknown. + } else if (isArray(node)) { for (let i = 0; i < node.length; i++) { const child = node[i]; if (isValidElement(child)) { @@ -180,7 +184,7 @@ function validateChildKeys(node, parentType) { if (node._store) { node._store.validated = true; } - } else if (node) { + } else { const iteratorFn = getIteratorFn(node); if (typeof iteratorFn === 'function') { // Entry iterators used to provide implicit keys, @@ -210,6 +214,9 @@ function validatePropTypes(element) { if (type === null || type === undefined || typeof type === 'string') { return; } + if (type.$$typeof === REACT_CLIENT_REFERENCE) { + return; + } let propTypes; if (typeof type === 'function') { propTypes = type.propTypes; diff --git a/packages/react/src/jsx/ReactJSXElementValidator.js b/packages/react/src/jsx/ReactJSXElementValidator.js index 947d806058732..da000079ee90f 100644 --- a/packages/react/src/jsx/ReactJSXElementValidator.js +++ b/packages/react/src/jsx/ReactJSXElementValidator.js @@ -32,6 +32,8 @@ import ReactSharedInternals from 'shared/ReactSharedInternals'; const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner; const ReactDebugCurrentFrame = ReactSharedInternals.ReactDebugCurrentFrame; +const REACT_CLIENT_REFERENCE = Symbol.for('react.client.reference'); + function setCurrentlyValidatingElement(element) { if (__DEV__) { if (element) { @@ -179,10 +181,12 @@ function validateExplicitKey(element, parentType) { */ function validateChildKeys(node, parentType) { if (__DEV__) { - if (typeof node !== 'object') { + if (typeof node !== 'object' || !node) { return; } - if (isArray(node)) { + if (node.$$typeof === REACT_CLIENT_REFERENCE) { + // This is a reference to a client component so it's unknown. + } else if (isArray(node)) { for (let i = 0; i < node.length; i++) { const child = node[i]; if (isValidElement(child)) { @@ -194,7 +198,7 @@ function validateChildKeys(node, parentType) { if (node._store) { node._store.validated = true; } - } else if (node) { + } else { const iteratorFn = getIteratorFn(node); if (typeof iteratorFn === 'function') { // Entry iterators used to provide implicit keys, @@ -225,6 +229,9 @@ function validatePropTypes(element) { if (type === null || type === undefined || typeof type === 'string') { return; } + if (type.$$typeof === REACT_CLIENT_REFERENCE) { + return; + } let propTypes; if (typeof type === 'function') { propTypes = type.propTypes;