Skip to content

Commit

Permalink
Revert the outer module object to an object (#26093)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sebmarkbage authored Feb 2, 2023
1 parent 9d111ff commit 922dd7b
Show file tree
Hide file tree
Showing 4 changed files with 122 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,94 @@ describe('ReactFlightDOM', () => {
expect(container.innerHTML).toBe('<p>@div</p>');
});

// @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 <p>{use(response)}</p>;
}

function App({response}) {
return (
<Suspense fallback={<h1>Loading...</h1>}>
<Print response={response} />
</Suspense>
);
}

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(
<Component greeting={hi} />,
webpackMap,
);
pipe(writable);
const response = ReactServerDOMReader.createFromReadableStream(readable);

const container = document.createElement('div');
const root = ReactDOMClient.createRoot(container);
await act(async () => {
root.render(<App response={response} />);
});
expect(container.innerHTML).toBe('<p>Hello World</p>');
});

// @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 <p>{use(response)}</p>;
}

function App({response}) {
return (
<Suspense fallback={<h1>Loading...</h1>}>
<Print response={response} />
</Suspense>
);
}

const {Component} = clientExports(Module);

const {writable, readable} = getTestStream();
const {pipe} = ReactServerDOMWriter.renderToPipeableStream(
<Component greeting={'Hello'} />,
webpackMap,
);
pipe(writable);
const response = ReactServerDOMReader.createFromReadableStream(readable);

const container = document.createElement('div');
const root = ReactDOMClient.createRoot(container);
await act(async () => {
root.render(<App response={response} />);
});
expect(container.innerHTML).toBe('<p>Hello World</p>');
});

// @gate enableUseHook
it('should unwrap async module references', async () => {
const AsyncModule = Promise.resolve(function AsyncModule({text}) {
Expand Down
13 changes: 10 additions & 3 deletions packages/react/src/ReactElementValidator.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)) {
Expand All @@ -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,
Expand Down Expand Up @@ -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;
Expand Down
13 changes: 10 additions & 3 deletions packages/react/src/jsx/ReactJSXElementValidator.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)) {
Expand All @@ -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,
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 922dd7b

Please sign in to comment.