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

[RFC] Use rollup to build the code and generate CommonJS and UMD version #43

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

realityking
Copy link
Contributor

@realityking realityking commented Nov 11, 2018

This is probably quite a bit to swallow, so quick overview what this PR does:

  • It replaces most of what's happening in build.js with rollup.
  • It builds slugify in 2 versions: UMD (same as before), CommonJS
  • It changes package.json to specific the CommonJS file as main

The benefit of all this trouble are bundle size and easier static analysis for IDEs and other tools. The UMD version is 40 bytes smaller when minified with UglifyJS (~1%), the new CommonJS version is even 238 bytes smaller (~4.2%). Static analysis for IDEs and tools like rollup becomes easier because they generally can't figure out what's exported by an UMD module.

It's also a good step towards ESM support but that will require a bit more work.

What do you think?

@realityking realityking force-pushed the rollup branch 2 times, most recently from bf6c71f to 911d739 Compare December 18, 2018 14:13
@realityking realityking force-pushed the rollup branch 2 times, most recently from 0324e13 to e9617c7 Compare February 25, 2019 10:20
@realityking realityking changed the title [RFC] Use rollup to build the code and generate CommonJS and ESM versions [RFC] Use rollup to build the code and generate CommonJS and UMD version Sep 27, 2020
@realityking
Copy link
Contributor Author

I've been playing grave digger and updated this PR to account for the refactoring since. They actually made this significantly easier as the new structure is much better suited for rollup.

Please let me know what you think.

@simov
Copy link
Owner

simov commented Oct 13, 2020

Thanks @realityking, I'll have a look at it.

@simov
Copy link
Owner

simov commented Oct 25, 2020

@realityking your PR is looking great, thanks for your work on it. However, I don't like the fact that building the module is now mandatory, besides injecting the charmap. Lets keep it open for now, keeping it up to date will be very easy.

@realityking realityking force-pushed the rollup branch 2 times, most recently from 5551cf8 to 76482da Compare November 24, 2020 14:24
@realityking
Copy link
Contributor Author

I pushed this a little further by also adding an ESM build.

@realityking realityking force-pushed the rollup branch 2 times, most recently from f3be5ad to 7a4713d Compare April 12, 2021 19:05
@realityking realityking force-pushed the rollup branch 4 times, most recently from cb00c63 to 83aba67 Compare January 18, 2025 17:35
@realityking
Copy link
Contributor Author

Rebased this (ancient) PR onto current master. The only other change is adding a tmp folder to avoid a 2nd copy of locales.json to be in the repo.

The rollup version is ancient as it's the last one to support Node.js 8. CI would have to move to 14+ or better 18+ for newer versions.

Happy to flesh this out into full ESM support if you're ok with the approach @simov @stscoundrel

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.

2 participants