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

Simplify Variants #8

Merged
merged 1 commit into from
Sep 16, 2021
Merged

Conversation

developit
Copy link
Contributor

The ES5 "compat" version of safari-14-idb-fix was actually 2b smaller than the default (modern) version. Since the differences between the two were limited to "function" vs arrows and const vs var, there should also be no measurable performance tradeoff. Simplifying the package by shipping only ES5 will help ease usage in bundlers/runtimes with varying support for Package Exports.

The new directory layout is:

dist/
    index.js  <- ESM
    index.cjs  <- CJS
    iife.min.js  <- IIFE
    index.d.ts  <- types

All are ES5, all are minified via Terser.

if (id.startsWith('@babel/runtime')) return true;
return {
input: 'src/index.ts',
plugins: [simpleTS('./'), terser()],
Copy link
Owner

Choose a reason for hiding this comment

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

Would you recommend minifying here? I tend to only minify the iife version, since it's the one likely to be used without a bundler.

It's a bit of a pain during dev if you end up with something throwing in minified code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TBH there are good logical arguments in both directions. My main reason for minifying was just to strip out all the comments. One option that might be a nice middle ground would be:

Suggested change
plugins: [simpleTS('./'), terser()],
plugins: [
simpleTS('./'),
terser({
mangle: false, // preserve all names
output: {
beautify: true // normalize/preserve whitespace
}
})
],

@jakearchibald
Copy link
Owner

I'm going to move terser to only operate on the iife, but I'll do that in a follow-up commit.

@jakearchibald jakearchibald merged commit 5b1f50d into jakearchibald:main Sep 16, 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.

2 participants