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
feat: replace precinct with esbuild #659
Conversation
⏱ Benchmark resultsComparing with c28eaaf largeDepsEsbuild: 10.4s⬇️ 11.70% decrease vs. c28eaaf
Legend
largeDepsZisi: 53s⬇️ 16.78% decrease vs. c28eaaf
Legend
|
package.json
Outdated
@@ -76,7 +77,6 @@ | |||
"p-map": "^4.0.0", | |||
"path-exists": "^4.0.0", | |||
"pkg-dir": "^5.0.0", | |||
"precinct": "^8.0.0", |
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.
🪦
src/runtimes/node/list_imports.js
Outdated
name: 'list-imports', | ||
setup(build) { | ||
build.onResolve({ filter: /.*/ }, (args) => { | ||
const isImport = args.path !== path && !isBuiltinModule(args.path) |
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.
precinct
has the includeCore
property to exclude built-in Node modules, which doesn't exist in esbuild. So we use is-builtin-module
to that instead.
|
||
return { | ||
namespace: 'list-imports', | ||
external: isImport, |
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.
By setting a path as external
we prevent the parser from traversing it.
// TODO: `precinct.paperwork()` uses `fs.readFileSync()` under the hood, | ||
// but should use `fs.readFile()` instead | ||
const dependencies = precinct.paperwork(path, { includeCore: false }) | ||
const dependencies = featureFlags.parseWithEsbuild |
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.
We'll use this feature flag to roll this out progressively. Once the rollout is complete, we can remove precinct
entirely.
Co-authored-by: ehmicky <ehmicky@users.noreply.github.com>
- Summary
Replaces
precinct
with esbuild, purely for its parsing capabilities, in the legacy bundler flow.Closes https://github.com/netlify/pod-serverless/issues/39.
- Test plan
N/A
- A picture of a cute animal (not mandatory but encouraged)