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

put babel and browserslist to better use #631

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from

Conversation

8633brown
Copy link
Contributor

@8633brown 8633brown commented Feb 28, 2021

maybe more of a proof of concept at the moment but i'm unsure as to whether the builds can be reduced in size any more.

Ive added corejs and setup babel/preset-env to actually be used to polyfill for unsupported browsers. I've used defaults, not ie 11 so you can see how easy it is to modify which browsers are supported or not. currently defaults supports ie 11. So removing the , not ie 11 will compile with support for for ie 11.

I've left the debug flag in so you can see where babel is adding its polyfills. when using , not ie 11 there are far fewer polyfills being added which makes it a little easier to follow whats being added where instead of being cluttered by the ie 11 polyfills.

currently from what i can see the build sizes would go like this:

build:basic build:basic-min
original (no preset-env) 950KiB 405 KiB
*chrome 86 946KiB 405KiB
default, no ie 11 1.15MiB 454KiB
defaults 1.38MiB 488KiB
defaults, ie >= 9, safari >= 5.1 1.41Mib 492KiB

* just to prove babel will use no polyfills on a browser which supports everything.

@8633brown
Copy link
Contributor Author

babel targets property has been changed to defaults, ie >= 9, safari >= 5.1.
I have updated the table to include this in the original comment

@paulrosen
Copy link
Owner

What's the status of this?

@8633brown
Copy link
Contributor Author

I'll fix the conflicts when I next look at it again.
It should be pretty much ready to merge especially now the dist folder is included in unpkg

@8633brown
Copy link
Contributor Author

8633brown commented Mar 29, 2021

adds a legacy build which uses babel to keep support for older browsers.

This will be output to the dist/abcjs-legacy-min.js file.
Anyone using older browsers should use this build.
Anyone only needing modern support can use the current dist/abcjs-basic-min.js in any web browser.
Anyone using node + npm to package the project themselves should carry on using import abcjs from 'abcjs'

The modern file still uses babel to transpile the code base to help solve any cross browser discrepancies.

At the moment package.json .main points to index.js. This gives no compatability issues right now however as soon as any ES6 and or TypeScript is used PKG.main should be directed to point at the dist/abc-basic.js file.

@paulrosen
Copy link
Owner

I think the legacy build needs to be the default for the minified versions. Perhaps the new bundled library should be abcjs-modern-min.js.

For the node version I'm not sure. We have to pick one. Since node_modules are not run through babel, I think it needs to point to the legacy build. And probably have a second entry point for the modern version.

I'm ready to add typescript now.

A big thing that needs to work is the source map - so dev tools works.

@8633brown
Copy link
Contributor Author

I'm thinking it's maybe overkill providing two versions.

The difference between defaults and defaults, ie >= 9, safari >= 5.1 is only 4KiB.
I think it makes more sense to just use defaults, ie >= 9, safari >= 5.1 and try to improve the project size elsewhere and support only a single instance.
This makes it easier to drop support when you feel happy enough to do so and should keep the typescript build system simpler too as well as keeping the build times down.

@paulrosen
Copy link
Owner

I agree - the extra build isn't worth it for that. Plus I bet that removing the shims that I put in over the years would be more than 4K.

And it should be easy to add typescript to the build right now - might as well do that so there is only one round of testing.

@8633brown 8633brown force-pushed the babel branch 2 times, most recently from a79e649 to bffcf47 Compare May 27, 2021 15:25
@8633brown
Copy link
Contributor Author

I've set up typescript in this now. Everything seems to still be working fine.

So now babel will transpile all .js files and any .ts ts-loader will handle.
The only thing I'm not sure about is how typescript handles core-js transforms.
But I dont think thats is too much of an issue right now as core-js hasn't been used before this PR so I can look into that in the future.

@paulrosen
Copy link
Owner

I'm revisiting this and doing some testing. It's not good that the package size jumps 80K for the polyfills when I know from browser testing they aren't necessary. And if the user has some other js loaded they might already have the polyfills.

I'm doing some experiments...

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.

None yet

2 participants