Skip to content
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

Merged
merged 8 commits into from Sep 20, 2021
Merged

feat: replace precinct with esbuild #659

merged 8 commits into from Sep 20, 2021

Conversation

eduardoboucas
Copy link
Member

@eduardoboucas eduardoboucas commented Sep 17, 2021

- 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)

gophers-1924099_1920-1-1920x960

@eduardoboucas eduardoboucas added the type: feature code contributing to the implementation of a feature and/or user facing functionality label Sep 17, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Sep 17, 2021

⏱ Benchmark results

Comparing with c28eaaf

largeDepsEsbuild: 10.4s

⬇️ 11.70% decrease vs. c28eaaf

^                  13.9s   13.9s                           14.1s   14.2s                                  
│  13.7s            ┌──┐    ┌──┐                            ┌──┐    ┌──┐   13.5s   13.2s                  
│   ┌──┐            |  |    |  |                   12.8s    |  |    |  |    ┌──┐    ┌──┐                  
│ ──┼──┼────────────┼──┼────┼──┼────────────────────┌──┐────┼──┼────┼──┼────┼──┼────┼──┼──────────────────
│   |  |            |  |    |  |   11.4s            |  |    |  |    |  |    |  |    |  |   11.1s          
│   |  |            |  |    |  |    ┌──┐            |  |    |  |    |  |    |  |    |  |    ┌──┐   10.4s  
│   |  |   10.3s    |  |    |  |    |  |   10.1s    |  |    |  |    |  |    |  |    |  |    |  |    ┌──┐  
│   |  |    ┌──┐    |  |    |  |    |  |    ┌──┐    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
└───┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴──>
    T-12    T-11    T-10    T-9     T-8     T-7     T-6     T-5     T-4     T-3     T-2     T-1      T    
Legend

largeDepsZisi: 53s

⬇️ 16.78% decrease vs. c28eaaf

^                 1m 13.5s1m 14.3s                                1m 14.8s                                
│ 1m 11.6s          ┌──┐    ┌──┐                          1m 10.9s  ┌──┐  1m 11.3s1m 9.5s                 
│   ┌──┐            |  |    |  |                  1m 7.8s   ┌──┐    |  |    ┌──┐    ┌──┐                  
│   |  |            |  |    |  |                    ┌──┐    |  |    |  |    |  |    |  |  1m 2.7s         
│ ──┼──┼────────────┼──┼────┼──┼────────────────────┼──┼────┼──┼────┼──┼────┼──┼────┼──┼────┌──┐──────────
│   |  |            |  |    |  |   57.6s            |  |    |  |    |  |    |  |    |  |    |  |          
│   |  |   52.9s    |  |    |  |    ┌──┐   50.8s    |  |    |  |    |  |    |  |    |  |    |  |    53s   
│   |  |    ┌──┐    |  |    |  |    |  |    ┌──┐    |  |    |  |    |  |    |  |    |  |    |  |    ┌──┐  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
└───┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴──>
    T-12    T-11    T-10    T-9     T-8     T-7     T-6     T-5     T-4     T-3     T-2     T-1      T    
Legend

@eduardoboucas eduardoboucas changed the title refactor: replace precinct with esbuild feat: replace precinct with esbuild Sep 17, 2021
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",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🪦

@eduardoboucas eduardoboucas marked this pull request as ready for review September 20, 2021 09:06
name: 'list-imports',
setup(build) {
build.onResolve({ filter: /.*/ }, (args) => {
const isImport = args.path !== path && !isBuiltinModule(args.path)
Copy link
Member Author

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,
Copy link
Member Author

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
Copy link
Member Author

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>
@kodiakhq kodiakhq bot merged commit 5bf51d0 into main Sep 20, 2021
@kodiakhq kodiakhq bot deleted the feat/remove-precinct branch September 20, 2021 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge type: feature code contributing to the implementation of a feature and/or user facing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants