Skip to content

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