Skip to content

Commit

Permalink
[Flight] Fallback to importing the whole module instead of encoding e…
Browse files Browse the repository at this point in the history
…very name (facebook#26624)

We currently don't just "require" a module by its module id/path. We
encode the pair of module id/path AND its export name. That's because
with module splitting, a single original module can end up in two or
more separate modules by name. Therefore the manifest files need to
encode how to require the whole module as well as how to require each
export name.

In practice, we don't currently use this because we end up forcing
Webpack to deopt and keep it together as a single module, and we don't
even have the code in the Webpack plugin to write separate values for
each export name.

The problem is with CJS we don't statically know what all the export
names will be. Since these cases will never be module split, we don't
really need to know.

This changes the Flight requires to first look for the specific name
we're loading and then if that name doesn't exist in the manifest we
fallback to looking for the `"*"` name containing the entire module and
look for the name in there at runtime.

We could probably optimize this a bit if we assume that CJS modules on
the server never get built with a name. That way we don't have to do the
failed lookup.

Additionally, since we've recently merged filepath + name into a single
string instead of two values, we now have to split those back out by
parsing the string. This is especially unfortunate for server references
since those should really not reveal what they are but be a hash or
something. The solution might just be to split them back out into two
separate fields again.

cc @shuding
  • Loading branch information
sebmarkbage authored and AndyPengc12 committed Apr 15, 2024
1 parent 189f8f3 commit 921d08d
Show file tree
Hide file tree
Showing 7 changed files with 263 additions and 52 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,29 @@ export function resolveClientReference<T>(
bundlerConfig: SSRManifest,
metadata: ClientReferenceMetadata,
): ClientReference<T> {
const resolvedModuleData = bundlerConfig[metadata.id][metadata.name];
return resolvedModuleData;
const moduleExports = bundlerConfig[metadata.id];
let resolvedModuleData = moduleExports[metadata.name];
let name;
if (resolvedModuleData) {
// The potentially aliased name.
name = resolvedModuleData.name;
} else {
// If we don't have this specific name, we might have the full module.
resolvedModuleData = moduleExports['*'];
if (!resolvedModuleData) {
throw new Error(
'Could not find the module "' +
metadata.id +
'" in the React SSR Manifest. ' +
'This is probably a bug in the React Server Components bundler.',
);
}
name = metadata.name;
}
return {
specifier: resolvedModuleData.specifier,
name: name,
};
}

export function resolveServerReference<T>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,17 +40,31 @@ export function resolveClientReference<T>(
metadata: ClientReferenceMetadata,
): ClientReference<T> {
if (bundlerConfig) {
const resolvedModuleData = bundlerConfig[metadata.id][metadata.name];
if (metadata.async) {
return {
id: resolvedModuleData.id,
chunks: resolvedModuleData.chunks,
name: resolvedModuleData.name,
async: true,
};
const moduleExports = bundlerConfig[metadata.id];
let resolvedModuleData = moduleExports[metadata.name];
let name;
if (resolvedModuleData) {
// The potentially aliased name.
name = resolvedModuleData.name;
} else {
return resolvedModuleData;
// If we don't have this specific name, we might have the full module.
resolvedModuleData = moduleExports['*'];
if (!resolvedModuleData) {
throw new Error(
'Could not find the module "' +
metadata.id +
'" in the React SSR Manifest. ' +
'This is probably a bug in the React Server Components bundler.',
);
}
name = metadata.name;
}
return {
id: resolvedModuleData.id,
chunks: resolvedModuleData.chunks,
name: name,
async: !!metadata.async,
};
}
return metadata;
}
Expand All @@ -59,8 +73,37 @@ export function resolveServerReference<T>(
bundlerConfig: ServerManifest,
id: ServerReferenceId,
): ClientReference<T> {
// This needs to return async: true if it's an async module.
return bundlerConfig[id];
let name = '';
let resolvedModuleData = bundlerConfig[id];
if (resolvedModuleData) {
// The potentially aliased name.
name = resolvedModuleData.name;
} else {
// We didn't find this specific export name but we might have the * export
// which contains this name as well.
// TODO: It's unfortunate that we now have to parse this string. We should
// probably go back to encoding path and name separately on the client reference.
const idx = id.lastIndexOf('#');
if (idx !== -1) {
name = id.substr(idx + 1);
resolvedModuleData = bundlerConfig[id.substr(0, idx)];
}
if (!resolvedModuleData) {
throw new Error(
'Could not find the module "' +
id +
'" in the React Server Manifest. ' +
'This is probably a bug in the React Server Components bundler.',
);
}
}
// TODO: This needs to return async: true if it's an async module.
return {
id: resolvedModuleData.id,
chunks: resolvedModuleData.chunks,
name: name,
async: false,
};
}

// The chunk cache contains all the chunks we've preloaded so far.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,17 +58,37 @@ export function resolveClientReferenceMetadata<T>(
config: ClientManifest,
clientReference: ClientReference<T>,
): ClientReferenceMetadata {
const resolvedModuleData = config[clientReference.$$id];
if (clientReference.$$async) {
return {
id: resolvedModuleData.id,
chunks: resolvedModuleData.chunks,
name: resolvedModuleData.name,
async: true,
};
const modulePath = clientReference.$$id;
let name = '';
let resolvedModuleData = config[modulePath];
if (resolvedModuleData) {
// The potentially aliased name.
name = resolvedModuleData.name;
} else {
return resolvedModuleData;
// We didn't find this specific export name but we might have the * export
// which contains this name as well.
// TODO: It's unfortunate that we now have to parse this string. We should
// probably go back to encoding path and name separately on the client reference.
const idx = modulePath.lastIndexOf('#');
if (idx !== -1) {
name = modulePath.substr(idx + 1);
resolvedModuleData = config[modulePath.substr(0, idx)];
}
if (!resolvedModuleData) {
throw new Error(
'Could not find the module "' +
modulePath +
'" in the React Client Manifest. ' +
'This is probably a bug in the React Server Components bundler.',
);
}
}
return {
id: resolvedModuleData.id,
chunks: resolvedModuleData.chunks,
name: name,
async: !!clientReference.$$async,
};
}

export function getServerReferenceId<T>(
Expand Down
16 changes: 12 additions & 4 deletions packages/react-server-dom-webpack/src/ReactFlightWebpackPlugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -247,10 +247,6 @@ export default class ReactFlightWebpackPlugin {
return;
}

const moduleProvidedExports = compilation.moduleGraph
.getExportsInfo(module)
.getProvidedExports();

const href = pathToFileURL(module.resource).href;

if (href !== undefined) {
Expand All @@ -267,6 +263,13 @@ export default class ReactFlightWebpackPlugin {
specifier: href,
name: '*',
};

// TODO: If this module ends up split into multiple modules, then
// we should encode each the chunks needed for the specific export.
// When the module isn't split, it doesn't matter and we can just
// encode the id of the whole module. This code doesn't currently
// deal with module splitting so is likely broken from ESM anyway.
/*
clientManifest[href + '#'] = {
id,
chunks: chunkIds,
Expand All @@ -277,6 +280,10 @@ export default class ReactFlightWebpackPlugin {
name: '',
};
const moduleProvidedExports = compilation.moduleGraph
.getExportsInfo(module)
.getProvidedExports();
if (Array.isArray(moduleProvidedExports)) {
moduleProvidedExports.forEach(function (name) {
clientManifest[href + '#' + name] = {
Expand All @@ -290,6 +297,7 @@ export default class ReactFlightWebpackPlugin {
};
});
}
*/

ssrManifest[id] = ssrExports;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,45 @@ describe('ReactFlightDOM', () => {
expect(container.innerHTML).toBe('<p>Hello World</p>');
});

// @gate enableUseHook
it('should be able to render a module split named component export', async () => {
const Module = {
// This gets split into a separate module from the original one.
split: 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 {split: Component} = clientExports(Module);

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

const container = document.createElement('div');
const root = ReactDOMClient.createRoot(container);
await act(() => {
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
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,35 @@ describe('ReactFlightDOMBrowser', () => {
}

function requireServerRef(ref) {
const metaData = webpackServerMap[ref];
const mod = __webpack_require__(metaData.id);
if (metaData.name === '*') {
let name = '';
let resolvedModuleData = webpackServerMap[ref];
if (resolvedModuleData) {
// The potentially aliased name.
name = resolvedModuleData.name;
} else {
// We didn't find this specific export name but we might have the * export
// which contains this name as well.
// TODO: It's unfortunate that we now have to parse this string. We should
// probably go back to encoding path and name separately on the client reference.
const idx = ref.lastIndexOf('#');
if (idx !== -1) {
name = ref.substr(idx + 1);
resolvedModuleData = webpackServerMap[ref.substr(0, idx)];
}
if (!resolvedModuleData) {
throw new Error(
'Could not find the module "' +
ref +
'" in the React Client Manifest. ' +
'This is probably a bug in the React Server Components bundler.',
);
}
}
const mod = __webpack_require__(resolvedModuleData.id);
if (name === '*') {
return mod;
}
return mod[metaData.name];
return mod[name];
}

async function callServer(actionId, body) {
Expand Down Expand Up @@ -824,6 +847,52 @@ describe('ReactFlightDOMBrowser', () => {
expect(result).toBe('Hello HI');
});

it('can call a module split server function', async () => {
let actionProxy;

function Client({action}) {
actionProxy = action;
return 'Click Me';
}

function greet(text) {
return 'Hello ' + text;
}

const ServerModule = serverExports({
// This gets split into another module
split: greet,
});
const ClientRef = clientExports(Client);

const stream = ReactServerDOMServer.renderToReadableStream(
<ClientRef action={ServerModule.split} />,
webpackMap,
);

const response = ReactServerDOMClient.createFromReadableStream(stream, {
async callServer(ref, args) {
const body = await ReactServerDOMClient.encodeReply(args);
return callServer(ref, body);
},
});

function App() {
return use(response);
}

const container = document.createElement('div');
const root = ReactDOMClient.createRoot(container);
await act(() => {
root.render(<App />);
});
expect(container.innerHTML).toBe('Click Me');
expect(typeof actionProxy).toBe('function');

const result = await actionProxy('Split');
expect(result).toBe('Hello Split');
});

it('can bind arguments to a server reference', async () => {
let actionProxy;

Expand Down
Loading

0 comments on commit 921d08d

Please sign in to comment.