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

chore: update rollup and uglify and the build process #5096

Merged
merged 20 commits into from
Apr 19, 2018
Merged

Conversation

gkatsev
Copy link
Member

@gkatsev gkatsev commented Apr 13, 2018

This is a WIP PR that upgrades rollup and uglify to latest versions. It also rewrites the rollup config to take advantage of the newest features of rollup. Minification is also split up into a separate process, mostly to speed up the build. The total build type can go down from about 2.5 minutes to around 1 minute for all the generated javascript files.

TODO:

  • hook up npm run build
  • hook up npm start
  • remove old build/rollup.js file
  • verify tests
  • verify builds

@gkatsev
Copy link
Member Author

gkatsev commented Apr 17, 2018

When using rollup --watch, rollup sets up an alternative screen and hides all other output. I have made a PR to fix it rollup/rollup#2125, hopefully, with the current clearScreen watch option, when my PR is merged, just updating to it will fix it.

@gkatsev gkatsev changed the title WIP: chore: update rollup and uglify and the build process chore: update rollup and uglify and the build process Apr 17, 2018
@gkatsev gkatsev added needs: LGTM Needs one or more additional approvals major labels Apr 17, 2018
@brandonocasey
Copy link
Contributor

So I guess its faster to minify outside of rollup here?

@gkatsev
Copy link
Member Author

gkatsev commented Apr 18, 2018

yup, because it doesn't need to do a lot of work yet another time plus we're sharing the name cache between builds. But the total build time from old stuff to new stuff is maybe half.

@brandonocasey
Copy link
Contributor

is the output any smaller/bigger?

@gkatsev
Copy link
Member Author

gkatsev commented Apr 19, 2018

It's pretty much the same size

@gkatsev
Copy link
Member Author

gkatsev commented Apr 19, 2018

This has benefit of using a default config for rollup and makes the config a lot more maintainable without weird custom code.

@gkatsev gkatsev merged commit 97db94e into master Apr 19, 2018
@gkatsev gkatsev deleted the update-rollup branch April 19, 2018 17:24
@misteroneill misteroneill removed the needs: LGTM Needs one or more additional approvals label Apr 19, 2018
@gkatsev gkatsev mentioned this pull request Apr 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants