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

[DO-NOT-MERGE] RFC: use esbuild to significantly speedup building #4532

Closed
wants to merge 1 commit into from
Closed

[DO-NOT-MERGE] RFC: use esbuild to significantly speedup building #4532

wants to merge 1 commit into from

Conversation

SamChou19815
Copy link
Contributor

Motivation

This PR tries to use esbuild to compile all the JS code (including all the mdx-react produced js code). Since esbuild is written in Go with performance in mind, it results in a much faster build. Locally, the server build takes 1min for each locale without this change, and only 40s with this change.

I plan to keep this PR just about replacing babel and minimizer with esbuild to keep the changes small for now. I will start setup the logic for conditionally using esbuild after the webpack PR is merged.

Have you read the Contributing Guidelines on pull requests?

(Write your answer here.)

Test Plan

CI

Related PRs

Redo of #4487.

@netlify
Copy link

netlify bot commented Mar 29, 2021

@SamChou19815 SamChou19815 changed the title RFC: use esbuild to significantly speedup building [DO-NOT-MERGE] RFC: use esbuild to significantly speedup building Mar 29, 2021
@netlify
Copy link

netlify bot commented Mar 29, 2021

@github-actions
Copy link

github-actions bot commented Mar 29, 2021

⚡️ Lighthouse report for the changes in this PR:

Category Score
🟠 Performance 63
🟢 Accessibility 96
🟢 Best practices 100
🟢 SEO 100
🟢 PWA 95

Lighthouse ran on https://deploy-preview-4532--docusaurus-2.netlify.app/

@SamChou19815

This comment has been minimized.

@SamChou19815
Copy link
Contributor Author

New build size report from github actions. Managed to cut down 1 percent in the generated JS.

Size Change: -5.97 kB (-1%)

Total Size: 570 kB

Filename Size Change
website/build/assets/js/main.********.js 395 kB -5.97 kB (-1%)
ℹ️ View Unchanged
Filename Size Change
website/build/assets/css/styles.********.css 87.2 kB 0 B
website/build/blog/2017/12/14/introducing-docusaurus/index.html 61.1 kB 0 B
website/build/docs/introduction/index.html 235 B 0 B
website/build/index.html 26.1 kB 0 B

@slorber slorber marked this pull request as draft April 1, 2021 17:33
@slorber
Copy link
Collaborator

slorber commented May 10, 2021

@SamChou19815 was wondering if we shouldn't build some kind of plugin system for the build tools (or at least webpack js loader) somehow.

There's also this https://github.com/alangpierce/sucrase that reports being very fast, and looks not too hard to adopt.
We should probably not commit to just one transpiler but allow the user to switch easily from one another. We can't really predict which tool will be the best in 2-3 years.

Probably hard though to decouple Docusaurus from Webpack in the short term

@SamChou19815
Copy link
Contributor Author

@slorber Yes, I think it might be better to expose the loader customization ability for now. We can probably let user specify their custom loader in docusaurus config?

options: {
loader: 'tsx',
format: isServer ? 'cjs' : undefined,
target: 'es2017',
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe target node12 for isServer would more appropriate?

For browsers, it seems we can't use esbuild directly but it seems there is a way to convert browserlists to esbuild target: evanw/esbuild#121

@@ -3,6 +3,7 @@
"compilerOptions": {
"incremental": true,
"tsBuildInfoFile": "./lib/.tsbuildinfo",
"module": "esnext",
Copy link
Collaborator

Choose a reason for hiding this comment

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

make sense, does it also solve a bug related to ESBuild or can we add this in a separate PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't remember the context anymore. I remember it's initially added to fix some issue, but maybe the issue is no longer here. However, I think this change is good anyways since the theme-common package is designed for browser to consume.

@slorber slorber closed this May 14, 2021
@SamChou19815 SamChou19815 deleted the esbuild branch May 14, 2021 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants