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

Make experimental speed changes standard and add esbuild plugin #428

Closed
wants to merge 6 commits into from

Conversation

maier49
Copy link
Contributor

@maier49 maier49 commented Aug 25, 2020

Type: feature

The following has been addressed in the PR:

  • There is a related issue
  • All code has been formatted with prettier
  • Unit or Functional tests are included in the PR
  • schema.json has been updated appopriately

Description:
This moves the "experimental" speed features into the standard development build where possible, also adding esbuild to replace the ts-loader.

  • Has essentially no effect on the production build, but cuts dev build times in half for a large project (from about 85 seconds to about 45 seconds) .
  • On a fresh project from cli-create-app dev build times went from 9-10 seconds to 7-8 seconds.
  • Checked source maps for dev and dist builds and they seem fine.
  • On repeated builds, including the hard source webpack plugin initially cut times down but the build time grew continuously on every subsequent builds. If that issue can be tracked down (I didn't see anything in their issues) then it could cut these times down further
  • We were previously removing the postcss config when the experimental speed flag was on. However, we've enabled nesting selectors in our css config. So removing that broke builds that otherwise would be fine, either by failing in BTR or simply breaking styles in the app.

Resolves #217

@maier49 maier49 changed the title Performance changes Make experimental speed changes standard and add esbuild plugin Aug 25, 2020
@@ -123,7 +114,7 @@ window['${libraryName}'].base = '${base}'</script>`,
config.plugins.push(
new BuildTimeRender({
...args['build-time-render'],
sync: singleBundle,
sync: true,
Copy link
Member

Choose a reason for hiding this comment

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

Does BTR always need to be "sync" with these changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not positive whether BTR breaks if it's async for a single bundle. Since the dev build here is now always a single bundle I just converted all the single bundle conditionals to true.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

BTR is still async unless singleBundle is explicitly set to true for the dist build, if that wasn't clear.

@agubler
Copy link
Member

agubler commented May 11, 2021

Closing this PR, there is basic support for esbuild now and there are signifiant conflicts. However some of these changes will need to be considered when re-visiting the performance issue #217

@agubler agubler closed this May 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Performance audit
2 participants