Skip to content

Commit

Permalink
Simplify Webpack References by encoding file path + export name as si…
Browse files Browse the repository at this point in the history
…ngle id

We always look up these references in a map so it doesn't matter what their
value is. It could be a hash for example.

The loaders now encode a single $$id instead of filepath + name.

This changes the react-client-manifest to have a single level. The value
inside the map is still split into module id + export name because that's
what gets looked up in webpack.

The react-ssr-manifest is still two levels because that's a reverse lookup.
  • Loading branch information
sebmarkbage committed Mar 3, 2023
1 parent 5c633a4 commit 56580f4
Show file tree
Hide file tree
Showing 17 changed files with 116 additions and 153 deletions.
Binary file added fixtures/flight/public/favicon.ico
Binary file not shown.
4 changes: 2 additions & 2 deletions fixtures/flight/server/region.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,8 @@ app.post('/', bodyParser.text(), async function (req, res) {
const {renderToPipeableStream} = await import(
'react-server-dom-webpack/server'
);
const serverReference = JSON.parse(req.get('rsc-action'));
const {filepath, name} = serverReference;
const serverReference = req.get('rsc-action');
const [filepath, name] = serverReference.split('#');
const action = (await import(filepath))[name];
// Validate that this is actually a function we intended to expose and
// not the client trying to invoke arbitrary functions. In a real app,
Expand Down
2 changes: 1 addition & 1 deletion fixtures/flight/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ let data = ReactServerDOMReader.createFromFetch(
method: 'POST',
headers: {
Accept: 'text/x-component',
'rsc-action': JSON.stringify({filepath: id.id, name: id.name}),
'rsc-action': id,
},
body: JSON.stringify(args),
});
Expand Down
10 changes: 6 additions & 4 deletions packages/react-client/src/ReactFlightClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,7 @@ function createModelReject<T>(chunk: SomeChunk<T>): (error: mixed) => void {

function createServerReferenceProxy<A: Iterable<any>, T>(
response: Response,
metaData: any,
metaData: {id: any, bound: Thenable<Array<any>>},
): (...A) => Promise<T> {
const callServer = response._callServer;
const proxy = function (): Promise<T> {
Expand All @@ -482,12 +482,14 @@ function createServerReferenceProxy<A: Iterable<any>, T>(
const p = metaData.bound;
if (p.status === INITIALIZED) {
const bound = p.value;
return callServer(metaData, bound.concat(args));
return callServer(metaData.id, bound.concat(args));
}
// Since this is a fake Promise whose .then doesn't chain, we have to wrap it.
// TODO: Remove the wrapper once that's fixed.
return Promise.resolve(p).then(function (bound) {
return callServer(metaData, bound.concat(args));
return ((Promise.resolve(p): any): Promise<Array<any>>).then(function (
bound,
) {
return callServer(metaData.id, bound.concat(args));
});
};
return proxy;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export function resolveClientReferenceMetadata<T>(
export function resolveServerReferenceMetadata<T>(
config: BundlerConfig,
resource: ServerReference<T>,
): ServerReferenceMetadata {
): {id: ServerReferenceMetadata, bound: Promise<Array<any>>} {
throw new Error('Not implemented.');
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ export opaque type ClientReferenceMetadata = {
id: string,
chunks: Array<string>,
name: string,
async: boolean,
};

// eslint-disable-next-line no-unused-vars
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,7 @@ import {
close,
} from 'react-client/src/ReactFlightClientStream';

type CallServerCallback = <A, T>(
{filepath: string, name: string},
args: A,
) => Promise<T>;
type CallServerCallback = <A, T>(string, args: A) => Promise<T>;

export type Options = {
callServer?: CallServerCallback,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,32 +10,24 @@
import type {ReactModel} from 'react-server/src/ReactFlightServer';

type WebpackMap = {
[filepath: string]: {
[name: string]: ClientReferenceMetadata,
},
[id: string]: ClientReferenceMetadata,
};

export type BundlerConfig = WebpackMap;

export type ServerReference<T: Function> = T & {
$$typeof: symbol,
$$filepath: string,
$$name: string,
$$id: string,
$$bound: Array<ReactModel>,
};

export type ServerReferenceMetadata = {
id: string,
name: string,
bound: Promise<Array<ReactModel>>,
};
export type ServerReferenceMetadata = string;

// eslint-disable-next-line no-unused-vars
export type ClientReference<T> = {
$$typeof: symbol,
filepath: string,
name: string,
async: boolean,
$$id: string,
$$async: boolean,
};

export type ClientReferenceMetadata = {
Expand All @@ -53,12 +45,7 @@ const SERVER_REFERENCE_TAG = Symbol.for('react.server.reference');
export function getClientReferenceKey(
reference: ClientReference<any>,
): ClientReferenceKey {
return (
reference.filepath +
'#' +
reference.name +
(reference.async ? '#async' : '')
);
return reference.$$async ? reference.$$id + '#async' : reference.$$id;
}

export function isClientReference(reference: Object): boolean {
Expand All @@ -73,9 +60,8 @@ export function resolveClientReferenceMetadata<T>(
config: BundlerConfig,
clientReference: ClientReference<T>,
): ClientReferenceMetadata {
const resolvedModuleData =
config[clientReference.filepath][clientReference.name];
if (clientReference.async) {
const resolvedModuleData = config[clientReference.$$id];
if (clientReference.$$async) {
return {
id: resolvedModuleData.id,
chunks: resolvedModuleData.chunks,
Expand All @@ -90,10 +76,9 @@ export function resolveClientReferenceMetadata<T>(
export function resolveServerReferenceMetadata<T>(
config: BundlerConfig,
serverReference: ServerReference<T>,
): ServerReferenceMetadata {
): {id: ServerReferenceMetadata, bound: Promise<Array<any>>} {
return {
id: serverReference.$$filepath,
name: serverReference.$$name,
id: serverReference.$$id,
bound: Promise.resolve(serverReference.$$bound),
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,7 @@ function transformServerModule(
}
newSrc += 'Object.defineProperties(' + local + ',{';
newSrc += '$$typeof: {value: Symbol.for("react.server.reference")},';
newSrc += '$$filepath: {value: ' + JSON.stringify(url) + '},';
newSrc += '$$name: { value: ' + JSON.stringify(exported) + '},';
newSrc += '$$id: {value: ' + JSON.stringify(url + '#' + exported) + '},';
newSrc += '$$bound: { value: [] }';
newSrc += '});\n';
});
Expand Down Expand Up @@ -343,9 +342,8 @@ async function transformClientModule(
');';
}
newSrc += '},{';
newSrc += 'name: { value: ' + JSON.stringify(name) + '},';
newSrc += '$$typeof: {value: CLIENT_REFERENCE},';
newSrc += 'filepath: {value: ' + JSON.stringify(url) + '}';
newSrc += '$$id: {value: ' + JSON.stringify(url + '#' + name) + '}';
newSrc += '});\n';
}
return newSrc;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,7 @@ module.exports = function register() {
// $FlowFixMe[method-unbinding]
const args = Array.prototype.slice.call(arguments, 1);
newFn.$$typeof = SERVER_REFERENCE;
newFn.$$filepath = this.$$filepath;
newFn.$$name = this.$$name;
newFn.$$id = this.$$id;
newFn.$$bound = this.$$bound.concat(args);
}
return newFn;
Expand All @@ -44,14 +43,14 @@ module.exports = function register() {
// These names are a little too common. We should probably have a way to
// have the Flight runtime extract the inner target instead.
return target.$$typeof;
case 'filepath':
return target.filepath;
case '$$id':
return target.$$id;
case '$$async':
return target.$$async;
case 'name':
return target.name;
case 'displayName':
return undefined;
case 'async':
return target.async;
// We need to special case this because createElement reads it if we pass this
// reference.
case 'defaultProps':
Expand All @@ -69,20 +68,8 @@ module.exports = function register() {
`that itself renders a Client Context Provider.`,
);
}
let expression;
switch (target.name) {
case '':
// eslint-disable-next-line react-internal/safe-string-coercion
expression = String(name);
break;
case '*':
// eslint-disable-next-line react-internal/safe-string-coercion
expression = String(name);
break;
default:
// eslint-disable-next-line react-internal/safe-string-coercion
expression = String(target.name) + '.' + String(name);
}
// eslint-disable-next-line react-internal/safe-string-coercion
const expression = String(target.name) + '.' + String(name);
throw new Error(
`Cannot access ${expression} on the server. ` +
'You cannot dot into a client module from a server component. ' +
Expand All @@ -103,15 +90,13 @@ module.exports = function register() {
switch (name) {
// These names are read by the Flight runtime if you end up using the exports object.
case '$$typeof':
// These names are a little too common. We should probably have a way to
// have the Flight runtime extract the inner target instead.
return target.$$typeof;
case 'filepath':
return target.filepath;
case '$$id':
return target.$$id;
case '$$async':
return target.$$async;
case 'name':
return target.name;
case 'async':
return target.async;
// We need to special case this because createElement reads it if we pass this
// reference.
case 'defaultProps':
Expand All @@ -125,7 +110,7 @@ module.exports = function register() {
case '__esModule':
// Something is conditionally checking which export to use. We'll pretend to be
// an ESM compat module but then we'll check again on the client.
const moduleId = target.filepath;
const moduleId = target.$$id;
target.default = Object.defineProperties(
(function () {
throw new Error(
Expand All @@ -136,12 +121,11 @@ module.exports = function register() {
);
}: any),
{
$$typeof: {value: CLIENT_REFERENCE},
// This a placeholder value that tells the client to conditionally use the
// whole object or just the default export.
name: {value: ''},
$$typeof: {value: CLIENT_REFERENCE},
filepath: {value: target.filepath},
async: {value: target.async},
$$id: {value: target.$$id + '#'},
$$async: {value: target.$$async},
},
);
return true;
Expand All @@ -150,17 +134,15 @@ module.exports = function register() {
// Use a cached value
return target.then;
}
if (!target.async) {
if (!target.$$async) {
// If this module is expected to return a Promise (such as an AsyncModule) then
// we should resolve that with a client reference that unwraps the Promise on
// the client.

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},
$$id: {value: target.$$id},
$$async: {value: true},
});
const proxy = new Proxy(clientReference, proxyHandlers);

Expand All @@ -176,10 +158,9 @@ module.exports = function register() {
// If this is not used as a Promise but is treated as a reference to a `.then`
// export then we should treat it as a reference to that name.
{
name: {value: 'then'},
$$typeof: {value: CLIENT_REFERENCE},
filepath: {value: target.filepath},
async: {value: false},
$$id: {value: target.$$id + '#then'},
$$async: {value: false},
},
));
return then;
Expand All @@ -206,8 +187,8 @@ module.exports = function register() {
{
name: {value: name},
$$typeof: {value: CLIENT_REFERENCE},
filepath: {value: target.filepath},
async: {value: target.async},
$$id: {value: target.$$id + '#' + name},
$$async: {value: target.$$async},
},
);
cachedReference = target[name] = new Proxy(
Expand Down Expand Up @@ -284,11 +265,10 @@ module.exports = function register() {
if (useClient) {
const moduleId: string = (url.pathToFileURL(filename).href: any);
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},
// Represents the whole Module object instead of a particular import.
$$id: {value: moduleId},
$$async: {value: false},
});
// $FlowFixMe[incompatible-call] found when upgrading Flow
this.exports = new Proxy(clientReference, proxyHandlers);
Expand All @@ -306,10 +286,9 @@ module.exports = function register() {
if (typeof exports === 'function') {
// The module exports a function directly,
Object.defineProperties((exports: any), {
// Represents the whole Module object instead of a particular import.
$$typeof: {value: SERVER_REFERENCE},
$$filepath: {value: moduleId},
$$name: {value: '*'},
// Represents the whole Module object instead of a particular import.
$$id: {value: moduleId},
$$bound: {value: []},
});
} else {
Expand All @@ -320,8 +299,7 @@ module.exports = function register() {
if (typeof value === 'function') {
Object.defineProperties((value: any), {
$$typeof: {value: SERVER_REFERENCE},
$$filepath: {value: moduleId},
$$name: {value: key},
$$id: {value: moduleId + '#' + key},
$$bound: {value: []},
});
}
Expand Down
Loading

0 comments on commit 56580f4

Please sign in to comment.