Skip to content

Commit

Permalink
[Flight Plugin] Scan for "use client" (#26474)
Browse files Browse the repository at this point in the history
## Summary

Our toy webpack plugin for Server Components is pretty broken right now
because, now that `.client.js` convention is gone, it ends up adding
every single JS file it can find (including `node_modules`) as a
potential async dependency. Instead, it should only look for files with
the `'use client'` directive.

The ideal way is to implement this by bundling the RSC graph first.
Then, we would know which `'use client'` files were actually discovered
— and so there would be no point to scanning the disk for them. That's
how Next.js bundler does it.

We're not doing that here.

This toy plugin is very simple, and I'm not planning to do heavy
lifting. I'm just bringing it up to date with the convention. The change
is that we now read every file we discover (alas), bail if it has no
`'use client'`, and parse it if it does (to verify it's actually used as
a directive). I've changed to use `acorn-loose` because it's forgiving
of JSX (and likely TypeScript/Flow). Otherwise, this wouldn't work on
uncompiled source.

## Test plan

Verified I can get our initial Server Components Demo running after this
change. Previously, it would get stuck compiling and then emit thousands
of errors.

Also confirmed the fixture still works. (It doesn’t work correctly on
the first load after dev server starts, but that’s already the case on
main so seems unrelated.)
  • Loading branch information
gaearon authored Mar 30, 2023
1 parent 1a1d61f commit 1308e49
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 9 deletions.
2 changes: 1 addition & 1 deletion packages/react-server-dom-webpack/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@
"webpack": "^5.59.0"
},
"dependencies": {
"acorn": "^6.2.1",
"acorn-loose": "^8.3.0",
"neo-async": "^2.6.1",
"loose-envify": "^1.1.0"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
* @flow
*/

import * as acorn from 'acorn';
import * as acorn from 'acorn-loose';

type ResolveContext = {
conditions: Array<string>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
* @flow
*/

const acorn = require('acorn');
const acorn = require('acorn-loose');

const url = require('url');

Expand Down
71 changes: 67 additions & 4 deletions packages/react-server-dom-webpack/src/ReactFlightWebpackPlugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@

import {join} from 'path';
import {pathToFileURL} from 'url';

import asyncLib from 'neo-async';
import * as acorn from 'acorn-loose';

import ModuleDependency from 'webpack/lib/dependencies/ModuleDependency';
import NullDependency from 'webpack/lib/dependencies/NullDependency';
Expand Down Expand Up @@ -117,10 +117,12 @@ export default class ReactFlightWebpackPlugin {
PLUGIN_NAME,
({contextModuleFactory}, callback) => {
const contextResolver = compiler.resolverFactory.get('context', {});
const normalResolver = compiler.resolverFactory.get('normal');

_this.resolveAllClientFiles(
compiler.context,
contextResolver,
normalResolver,
compiler.inputFileSystem,
contextModuleFactory,
function (err, resolvedClientRefs) {
Expand Down Expand Up @@ -219,6 +221,10 @@ export default class ReactFlightWebpackPlugin {
return;
}

const resolvedClientFiles = new Set(
(resolvedClientReferences || []).map(ref => ref.request),
);

const clientManifest: {
[string]: {chunks: $FlowFixMe, id: string, name: string},
} = {};
Expand All @@ -237,8 +243,7 @@ export default class ReactFlightWebpackPlugin {
// TODO: Hook into deps instead of the target module.
// That way we know by the type of dep whether to include.
// It also resolves conflicts when the same module is in multiple chunks.

if (!/\.(js|ts)x?$/.test(module.resource)) {
if (!resolvedClientFiles.has(module.resource)) {
return;
}

Expand Down Expand Up @@ -328,13 +333,39 @@ export default class ReactFlightWebpackPlugin {
resolveAllClientFiles(
context: string,
contextResolver: any,
normalResolver: any,
fs: any,
contextModuleFactory: any,
callback: (
err: null | Error,
result?: $ReadOnlyArray<ClientReferenceDependency>,
) => void,
) {
function hasUseClientDirective(source: string): boolean {
if (source.indexOf('use client') === -1) {
return false;
}
let body;
try {
body = acorn.parse(source, {
ecmaVersion: '2024',
sourceType: 'module',
}).body;
} catch (x) {
return false;
}
for (let i = 0; i < body.length; i++) {
const node = body[i];
if (node.type !== 'ExpressionStatement' || !node.directive) {
break;
}
if (node.directive === 'use client') {
return true;
}
}
return false;
}

asyncLib.map(
this.clientReferences,
(
Expand Down Expand Up @@ -373,14 +404,46 @@ export default class ReactFlightWebpackPlugin {
options,
(err2: null | Error, deps: Array<any /*ModuleDependency*/>) => {
if (err2) return cb(err2);

const clientRefDeps = deps.map(dep => {
// use userRequest instead of request. request always end with undefined which is wrong
const request = join(resolvedDirectory, dep.userRequest);
const clientRefDep = new ClientReferenceDependency(request);
clientRefDep.userRequest = dep.userRequest;
return clientRefDep;
});
cb(null, clientRefDeps);

asyncLib.filter(
clientRefDeps,
(
clientRefDep: ClientReferenceDependency,
filterCb: (err: null | Error, truthValue: boolean) => void,
) => {
normalResolver.resolve(
{},
context,
clientRefDep.request,
{},
(err3: null | Error, resolvedPath: mixed) => {
if (err3 || typeof resolvedPath !== 'string') {
return filterCb(null, false);
}
fs.readFile(
resolvedPath,
'utf-8',
(err4: null | Error, content: string) => {
if (err4 || typeof content !== 'string') {
return filterCb(null, false);
}
const useClient = hasUseClientDirective(content);
filterCb(null, useClient);
},
);
},
);
},
cb,
);
},
);
},
Expand Down
11 changes: 9 additions & 2 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3434,12 +3434,19 @@ acorn-jsx@^5.3.1:
resolved "https://registry.yarnpkg.com/acorn-jsx/-/acorn-jsx-5.3.2.tgz#7ed5bb55908b3b2f1bc55c6af1653bada7f07937"
integrity sha512-rq9s+JNhf0IChjtDXxllJ7g41oZk5SlXtp0LHwyA5cejwn7vKmKp4pPri6YEePv2PU65sAsegbXtIinmDFDXgQ==

acorn-loose@^8.3.0:
version "8.3.0"
resolved "https://registry.yarnpkg.com/acorn-loose/-/acorn-loose-8.3.0.tgz#0cd62461d21dce4f069785f8d3de136d5525029a"
integrity sha512-75lAs9H19ldmW+fAbyqHdjgdCrz0pWGXKmnqFoh8PyVd1L2RIb4RzYrSjmopeqv3E1G3/Pimu6GgLlrGbrkF7w==
dependencies:
acorn "^8.5.0"

acorn-walk@^8.0.2:
version "8.2.0"
resolved "https://registry.yarnpkg.com/acorn-walk/-/acorn-walk-8.2.0.tgz#741210f2e2426454508853a2f44d0ab83b7f69c1"
integrity sha512-k+iyHEuPgSw6SbuDpGQM+06HQUa04DZ3o+F6CSzXMvvI5KMvnaEqXe+YVe555R9nn6GPt404fos4wcgpw12SDA==

acorn@^6.0.7, acorn@^6.2.1, acorn@^6.4.1:
acorn@^6.0.7, acorn@^6.4.1:
version "6.4.2"
resolved "https://registry.yarnpkg.com/acorn/-/acorn-6.4.2.tgz#35866fd710528e92de10cf06016498e47e39e1e6"
integrity sha512-XtGIhXwF8YM8bJhGxG5kXgjkEuNGLTkoYqVE+KMR+aspr4KGYmKYg7yUe3KghyQ9yheNwLnjmzh/7+gfDBmHCQ==
Expand All @@ -3449,7 +3456,7 @@ acorn@^7.1.1, acorn@^7.4.0:
resolved "https://registry.yarnpkg.com/acorn/-/acorn-7.4.1.tgz#feaed255973d2e77555b83dbc08851a6c63520fa"
integrity sha512-nQyp0o1/mNdbTO1PO6kHkwSrmgZ0MT/jCCpNiwbUjGoRN4dlBhqJtoQuCnEOKzgTVwg0ZWiCoQy6SxMebQVh8A==

acorn@^8.1.0, acorn@^8.8.1:
acorn@^8.1.0, acorn@^8.5.0, acorn@^8.8.1:
version "8.8.2"
resolved "https://registry.yarnpkg.com/acorn/-/acorn-8.8.2.tgz#1b2f25db02af965399b9776b0c2c391276d37c4a"
integrity sha512-xjIYgE8HBrkpd/sJqOGNspf8uHG+NOHGOw6a/Urj8taM2EXfdNAH2oFcPeIFfsv3+kz/mJrS5VuMqbNLjCa2vw==
Expand Down

0 comments on commit 1308e49

Please sign in to comment.