-
Notifications
You must be signed in to change notification settings - Fork 47k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Flight] Basic scan of the file system to find Client modules #20383
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,17 +8,169 @@ | |
*/ | ||
|
||
import {mkdirSync, writeFileSync} from 'fs'; | ||
import {dirname, resolve} from 'path'; | ||
import {dirname, resolve, join} from 'path'; | ||
import {pathToFileURL} from 'url'; | ||
|
||
import asyncLib from 'neo-async'; | ||
|
||
import ModuleDependency from 'webpack/lib/dependencies/ModuleDependency'; | ||
import NullDependency from 'webpack/lib/dependencies/NullDependency'; | ||
import AsyncDependenciesBlock from 'webpack/lib/AsyncDependenciesBlock'; | ||
import Template from 'webpack/lib/Template'; | ||
|
||
class ClientReferenceDependency extends ModuleDependency { | ||
constructor(request) { | ||
super(request); | ||
} | ||
|
||
get type() { | ||
return 'client-reference'; | ||
} | ||
} | ||
|
||
// This is the module that will be used to anchor all client references to. | ||
// I.e. it will have all the client files as async deps from this point on. | ||
// We use the Flight client implementation because you can't get to these | ||
// without the client runtime so it's the first time in the loading sequence | ||
// you might want them. | ||
const clientFileName = require.resolve('../'); | ||
|
||
type ClientReferenceSearchPath = { | ||
directory: string, | ||
recursive?: boolean, | ||
include: RegExp, | ||
exclude?: RegExp, | ||
}; | ||
|
||
type ClientReferencePath = string | ClientReferenceSearchPath; | ||
|
||
type Options = { | ||
isServer: boolean, | ||
clientReferences?: ClientReferencePath | $ReadOnlyArray<ClientReferencePath>, | ||
chunkName?: string, | ||
}; | ||
|
||
const PLUGIN_NAME = 'React Transport Plugin'; | ||
|
||
export default class ReactFlightWebpackPlugin { | ||
constructor(options: {isServer: boolean}) {} | ||
clientReferences: $ReadOnlyArray<ClientReferencePath>; | ||
chunkName: string; | ||
constructor(options: Options) { | ||
if (!options || typeof options.isServer !== 'boolean') { | ||
throw new Error( | ||
PLUGIN_NAME + ': You must specify the isServer option as a boolean.', | ||
); | ||
} | ||
if (options.isServer) { | ||
throw new Error('TODO: Implement the server compiler.'); | ||
} | ||
if (!options.clientReferences) { | ||
this.clientReferences = [ | ||
{ | ||
directory: '.', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is this? Project cwd? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It’s relative to Webpack’s virtual file system. So it’s whatever that is. Nothing touches a real file system directly. |
||
recursive: true, | ||
include: /\.client\.(js|ts|jsx|tsx)$/, | ||
}, | ||
]; | ||
} else if ( | ||
typeof options.clientReferences === 'string' || | ||
!Array.isArray(options.clientReferences) | ||
) { | ||
this.clientReferences = [(options.clientReferences: $FlowFixMe)]; | ||
} else { | ||
this.clientReferences = options.clientReferences; | ||
} | ||
if (typeof options.chunkName === 'string') { | ||
this.chunkName = options.chunkName; | ||
if (!/\[(index|request)\]/.test(this.chunkName)) { | ||
this.chunkName += '[index]'; | ||
} | ||
} else { | ||
this.chunkName = 'client[index]'; | ||
} | ||
} | ||
|
||
apply(compiler: any) { | ||
compiler.hooks.emit.tap('React Transport Plugin', compilation => { | ||
const run = (params, callback) => { | ||
// First we need to find all client files on the file system. We do this early so | ||
// that we have them synchronously available later when we need them. This might | ||
// not be needed anymore since we no longer need to compile the module itself in | ||
// a special way. So it's probably better to do this lazily and in parallel with | ||
// other compilation. | ||
const contextResolver = compiler.resolverFactory.get('context', {}); | ||
this.resolveAllClientFiles( | ||
compiler.context, | ||
contextResolver, | ||
compiler.inputFileSystem, | ||
compiler.createContextModuleFactory(), | ||
(err, resolvedClientReferences) => { | ||
if (err) { | ||
callback(err); | ||
return; | ||
} | ||
compiler.hooks.compilation.tap( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it safe to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe this is a blocking phase so Webpack doesn’t proceed until we’ve invoked the callback. That’s also why this approach is suboptimal since it doesn’t allow for other parallelism in the meantime. That was a limitation in the previous approach that needed to change/split the files. We could put this elsewhere but it’s not broken. |
||
PLUGIN_NAME, | ||
(compilation, {normalModuleFactory}) => { | ||
compilation.dependencyFactories.set( | ||
ClientReferenceDependency, | ||
normalModuleFactory, | ||
); | ||
compilation.dependencyTemplates.set( | ||
ClientReferenceDependency, | ||
new NullDependency.Template(), | ||
); | ||
|
||
compilation.hooks.buildModule.tap(PLUGIN_NAME, module => { | ||
// We need to add all client references as dependency of something in the graph so | ||
// Webpack knows which entries need to know about the relevant chunks and include the | ||
// map in their runtime. The things that actually resolves the dependency is the Flight | ||
// client runtime. So we add them as a dependency of the Flight client runtime. | ||
// Anything that imports the runtime will be made aware of these chunks. | ||
// TODO: Warn if we don't find this file anywhere in the compilation. | ||
if (module.resource !== clientFileName) { | ||
return; | ||
} | ||
if (resolvedClientReferences) { | ||
for (let i = 0; i < resolvedClientReferences.length; i++) { | ||
const dep = resolvedClientReferences[i]; | ||
const chunkName = this.chunkName | ||
.replace(/\[index\]/g, '' + i) | ||
.replace( | ||
/\[request\]/g, | ||
Template.toPath(dep.userRequest), | ||
); | ||
|
||
const block = new AsyncDependenciesBlock( | ||
{ | ||
name: chunkName, | ||
}, | ||
module, | ||
null, | ||
dep.require, | ||
); | ||
block.addDependency(dep); | ||
module.addBlock(block); | ||
} | ||
} | ||
}); | ||
}, | ||
); | ||
|
||
callback(); | ||
}, | ||
); | ||
}; | ||
|
||
compiler.hooks.run.tapAsync(PLUGIN_NAME, run); | ||
compiler.hooks.watchRun.tapAsync(PLUGIN_NAME, run); | ||
|
||
compiler.hooks.emit.tap(PLUGIN_NAME, compilation => { | ||
const json = {}; | ||
compilation.chunks.forEach(chunk => { | ||
chunk.getModules().forEach(mod => { | ||
// TOOD: 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 (!/\.client\.js$/.test(mod.resource)) { | ||
return; | ||
} | ||
|
@@ -42,7 +194,83 @@ export default class ReactFlightWebpackPlugin { | |
'react-transport-manifest.json', | ||
); | ||
mkdirSync(dirname(filename), {recursive: true}); | ||
// TODO: Use webpack's emit API and read from the devserver. | ||
writeFileSync(filename, output); | ||
}); | ||
} | ||
|
||
// This attempts to replicate the dynamic file path resolution used for other wildcard | ||
// resolution in Webpack is using. | ||
resolveAllClientFiles( | ||
context: string, | ||
contextResolver: any, | ||
fs: any, | ||
contextModuleFactory: any, | ||
callback: ( | ||
err: null | Error, | ||
result?: $ReadOnlyArray<ClientReferenceDependency>, | ||
) => void, | ||
) { | ||
asyncLib.map( | ||
this.clientReferences, | ||
( | ||
clientReferencePath: string | ClientReferenceSearchPath, | ||
cb: ( | ||
err: null | Error, | ||
result?: $ReadOnlyArray<ClientReferenceDependency>, | ||
) => void, | ||
): void => { | ||
if (typeof clientReferencePath === 'string') { | ||
cb(null, [new ClientReferenceDependency(clientReferencePath)]); | ||
return; | ||
} | ||
const clientReferenceSearch: ClientReferenceSearchPath = clientReferencePath; | ||
contextResolver.resolve( | ||
{}, | ||
context, | ||
clientReferencePath.directory, | ||
{}, | ||
(err, resolvedDirectory) => { | ||
if (err) return cb(err); | ||
const options = { | ||
resource: resolvedDirectory, | ||
resourceQuery: '', | ||
recursive: | ||
clientReferenceSearch.recursive === undefined | ||
? true | ||
: clientReferenceSearch.recursive, | ||
regExp: clientReferenceSearch.include, | ||
include: undefined, | ||
exclude: clientReferenceSearch.exclude, | ||
}; | ||
contextModuleFactory.resolveDependencies( | ||
fs, | ||
options, | ||
(err2: null | Error, deps: Array<ModuleDependency>) => { | ||
if (err2) return cb(err2); | ||
const clientRefDeps = deps.map(dep => { | ||
const request = join(resolvedDirectory, dep.request); | ||
const clientRefDep = new ClientReferenceDependency(request); | ||
clientRefDep.userRequest = dep.userRequest; | ||
return clientRefDep; | ||
}); | ||
cb(null, clientRefDeps); | ||
}, | ||
); | ||
}, | ||
); | ||
}, | ||
( | ||
err: null | Error, | ||
result: $ReadOnlyArray<$ReadOnlyArray<ClientReferenceDependency>>, | ||
): void => { | ||
if (err) return callback(err); | ||
const flat = []; | ||
for (let i = 0; i < result.length; i++) { | ||
flat.push.apply(flat, result[i]); | ||
} | ||
callback(null, flat); | ||
}, | ||
); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have a plan for
node_modules
? Am I supposed to exclude them here? Do they follow a different convention?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You’re supposed to include them. That’s why this is such a bad idea and it’s better to compile server first and use the reachable ones as a list here.
But there are tradeoffs and both approaches are valid.
This affords much more parallelism which is why we do something similar at FB. And we don’t have thousands of unused files in our graph.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'm still fuzzy on how you distribute Server+Client over npm. E.g. the default on npm today is Client. So if I import
react-something
from a Server one then I'd actually expect this to behave like a.client
entry point because the package author doesn't know about the distinction.If we were to start from scratch then defaulting to Server makes sense. But I don't know how this would work with the existing ecosystem?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You will get a static error on the server because you’re trying to import an export from React that doesn’t exist like useState. At least with real ESM. (We might want to fast track ESM for that reason.)
Existing things that just work as hybrids continue to work. Others will error on the server and you have to wrap it in a .client file. Ideally the publisher would’ve already done that but if not, you have to add a local wrapper to fix the error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that I think it’s the resolved file that needs to be .client.js. So you can add an “exports” alias so the import is just plain.
That’s no different than a hybrid wrapper that references a client.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makes sense