Add Babel with support for Node v12. #215

jgerigmeyer · 2022-09-09T16:15:20Z

Fixes #205.

This is related to #206 as well, but does not add in CI to test Node version support.

LeaVerou · 2022-09-09T16:45:39Z

rollup.config.js

@@ -1,44 +1,54 @@
+import { babel } from "@rollup/plugin-babel";
+import commonjs from "@rollup/plugin-commonjs";


I'm not sure why we need this one?

Because this is currently setup to use @babel/preset-env with useBuiltIns: 'usage', it automatically pulls in any necessary polyfills from core-js. Since we want to include those polyfills in the distributed bundles, we need to use this plugin to convert them from CommonJS modules to ES6, so Rollup can include them. See: https://github.com/rollup/plugins/tree/master/packages/node-resolve#using-with-rollupplugin-commonjs

I see, thanks. Does it only pull in polyfills that are actually needed or everything? Do you have any sense of how much this increases the size of the bundles?

Does it only pull in polyfills that are actually needed or everything?

It only pulls in the polyfills that are necessary for the target (node v12) based on the code actually used. In this case, I think it's these two small polyfills:

https://github.com/zloirock/core-js/blob/master/packages/core-js/modules/es.error.cause.js (used throughout)

https://github.com/zloirock/core-js/blob/master/packages/core-js/modules/es.reflect.to-string-tag.js (only used by space-accessors.js)

Do you have any sense of how much this increases the size of the bundles?

It increases the bundle size by about 44% (non-minified) or 30% (minified). The large majority of this increase is syntax re-writing by Babel, not the core-js polyfills themselves.

Before:

Created bundle color.global.js: 107.94 kB → 34.67 kB (gzip) Created bundle color.js: 104.35 kB → 34.23 kB (gzip) Created bundle color.cjs: 104.42 kB → 34.27 kB (gzip) Created bundle color.min.js: 41.59 kB → 16.48 kB (gzip) Created bundle color.global.min.js: 41.6 kB → 16.52 kB (gzip) Created bundle color.min.cjs: 41.66 kB → 16.5 kB (gzip)

After:

Created bundle color.global.js: 155.44 kB → 44.57 kB (gzip) Created bundle color.js: 151.04 kB → 44.22 kB (gzip) Created bundle color.cjs: 151.12 kB → 44.25 kB (gzip) Created bundle color.min.js: 54.88 kB → 21.45 kB (gzip) Created bundle color.global.min.js: 54.89 kB → 21.55 kB (gzip) Created bundle color.min.cjs: 54.95 kB → 21.48 kB (gzip)

If you're concerned about that, I suppose another option would be to add new bundles that have support for older versions of Node, and not run everything through Babel? If you'd like to go that route, I'm happy to adjust this PR.

Hey, so sorry for the delay, I didn't get a notification and only saw you had responded when I manually visited the PR to see if there were any updates. Ouch, that's quite an increase. Yes, I think the way to go would be to have separate bundles. In that case we could even support somewhat older browsers too in those bundles, what do you think?
Is there a good naming convention for the transpiled bundles that we could follow?

Yes, I think the way to go would be to have separate bundles. In that case we could even support somewhat older browsers too in those bundles, what do you think? Is there a good naming convention for the transpiled bundles that we could follow?

@LeaVerou I agree, that seems like a good way to go. Are you thinking multiple new bundles with different levels of browser/node support? Or one "legacy" bundle that has support for older platforms?

I think I'd lean toward the latter, and probably I'd propose something like color-legacy.js? 🤷

I'll do a quick update to try that out with Node 12 -- let me know if there are other platforms you think should be targeted (see: https://babeljs.io/docs/en/options#targets).

(The most recent commit doesn't work -- I'll push a fixed version shortly.)

@LeaVerou Adjusted in 862425c so that the original bundles are unchanged.

LeaVerou · 2022-09-09T16:46:22Z

Hey, thanks! See one question, apart from that LGTM.
In future PRs please do try to separate style changes from the actual PR, as it makes reviewing harder.

jgerigmeyer

In future PRs please do try to separate style changes from the actual PR, as it makes reviewing harder.

Reverted style changes and squashed commits. 👍

jgerigmeyer · 2022-09-09T17:34:21Z

rollup.config.js

@@ -1,44 +1,54 @@
+import { babel } from "@rollup/plugin-babel";
+import commonjs from "@rollup/plugin-commonjs";


Because this is currently setup to use @babel/preset-env with useBuiltIns: 'usage', it automatically pulls in any necessary polyfills from core-js. Since we want to include those polyfills in the distributed bundles, we need to use this plugin to convert them from CommonJS modules to ES6, so Rollup can include them. See: https://github.com/rollup/plugins/tree/master/packages/node-resolve#using-with-rollupplugin-commonjs

jgerigmeyer · 2022-09-16T13:05:19Z

Hi @LeaVerou, any thoughts on this? Thanks!

jgerigmeyer · 2022-09-28T15:08:07Z

Hi @LeaVerou, sorry to bother you, but I'm happy to wrap up this PR if you have any feedback? Thanks again!

LeaVerou · 2022-09-28T17:23:24Z

Sorry @jgerigmeyer, I had a deadline, then travel, and then got covid 🤦🏽‍♀️ I'm still in quarantine, but will try to review this soon. So sorry about the delay, please don't take this as meaning anything about how interested we are in merging this PR!

jgerigmeyer · 2022-09-28T17:25:09Z

Oh no! Please take your time, and feel better soon!

LeaVerou · 2022-09-29T00:05:59Z

Thank you!
I took a look and LGTM, my only comment would be since we're producing separate versions anyway, and doing all this work with Babel etc, we may as well add some more targets (older browsers that people may want to support) and produce a color.global.legacy.js build as well (or would it be color.legacy.global.js?)
(Not crazy old like IE11 though)

jgerigmeyer · 2022-09-30T16:03:20Z

Thank you! I took a look and LGTM, my only comment would be since we're producing separate versions anyway, and doing all this work with Babel etc, we may as well add some more targets (older browsers that people may want to support)

@LeaVerou Sure! I added the browserslist defaults in d008c58 -- this is the current list that it's supporting.

Do you have particular browsers in mind? I wasn't sure where to make the cutoff, and if we go much further back (e.g. Android 4 or iOS 12) then the bundles roughly double in size.

and produce a color.global.legacy.js build as well (or would it be color.legacy.global.js?) (Not crazy old like IE11 though)

Done -- here's the full list:

dist/color.global.js
dist/color.global.min.js
dist/color.js
dist/color.min.js
dist/color.cjs
dist/color.min.cjs
dist/color.global.legacy.js
dist/color.global.legacy.min.js
dist/color.legacy.js
dist/color.legacy.min.js
dist/color.legacy.cjs
dist/color.legacy.min.cjs

The current bundles are unchanged:

color.global.js: 107.94 kB → 34.67 kB (gzip)
color.js: 104.35 kB → 34.23 kB (gzip)
color.cjs: 104.42 kB → 34.27 kB (gzip)
color.min.cjs: 41.66 kB → 16.5 kB (gzip)
color.min.js: 41.59 kB → 16.48 kB (gzip)
color.global.min.js: 41.6 kB → 16.52 kB (gzip)

Here are the current file sizes of the new bundles:

color.global.legacy.js: 167.56 kB → 46.34 kB (gzip)
color.legacy.js: 158.47 kB → 45.85 kB (gzip)
color.legacy.cjs: 158.55 kB → 45.89 kB (gzip)
color.legacy.min.js: 58.2 kB → 22.27 kB (gzip)
color.legacy.min.cjs: 58.27 kB → 22.3 kB (gzip)
color.global.legacy.min.js: 58.22 kB → 22.37 kB (gzip)

For comparison, here's what it would be if we changed to add support for > 0.2% usage:

color.global.legacy.js: 313.81 kB → 80.41 kB (gzip)
color.legacy.js: 297.66 kB → 79.41 kB (gzip)
color.legacy.cjs: 297.74 kB → 79.44 kB (gzip)
color.legacy.min.js: 99.7 kB → 38.08 kB (gzip)
color.legacy.min.cjs: 99.77 kB → 38.1 kB (gzip)
color.global.legacy.min.js: 99.64 kB → 38.23 kB (gzip)

LeaVerou · 2022-09-30T18:10:46Z

LGTM, thank you for working on this! Just merged. :)

jgerigmeyer mentioned this pull request Sep 9, 2022

Replace TinyColor with Colorjs.io oddbird/sassdoc-theme-herman#384

Merged

LeaVerou reviewed Sep 9, 2022

View reviewed changes

Add Babel with support for Node v12.

251c423

jgerigmeyer force-pushed the node12 branch from 3adf6ca to 251c423 Compare September 9, 2022 17:29

jgerigmeyer commented Sep 9, 2022

View reviewed changes

jgerigmeyer added 2 commits September 12, 2022 12:26

Add separate legacy bundles for Node.js 12 support.

fb3c810

Fix legacy bundles.

862425c

jgerigmeyer requested a review from LeaVerou September 12, 2022 20:42

Add default browser support

d008c58

Upgrade deps

f428365

LeaVerou merged commit 50891cc into color-js:main Sep 30, 2022

jgerigmeyer deleted the node12 branch September 30, 2022 18:36

jgerigmeyer mentioned this pull request Oct 31, 2022

Legacy builds are not included in NPM package. #232

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Babel with support for Node v12. #215

Add Babel with support for Node v12. #215

jgerigmeyer commented Sep 9, 2022

LeaVerou Sep 9, 2022

jgerigmeyer Sep 9, 2022

LeaVerou Sep 9, 2022

jgerigmeyer Sep 9, 2022 •

edited

Loading

LeaVerou Sep 11, 2022

jgerigmeyer Sep 12, 2022

jgerigmeyer Sep 12, 2022

jgerigmeyer Sep 12, 2022

LeaVerou commented Sep 9, 2022

jgerigmeyer left a comment

jgerigmeyer Sep 9, 2022

jgerigmeyer commented Sep 16, 2022

jgerigmeyer commented Sep 28, 2022

LeaVerou commented Sep 28, 2022

jgerigmeyer commented Sep 28, 2022

LeaVerou commented Sep 29, 2022 •

edited

Loading

jgerigmeyer commented Sep 30, 2022 •

edited

Loading

LeaVerou commented Sep 30, 2022

		@@ -1,44 +1,54 @@
		import { babel } from "@rollup/plugin-babel";
		import commonjs from "@rollup/plugin-commonjs";

Add Babel with support for Node v12. #215

Add Babel with support for Node v12. #215

Conversation

jgerigmeyer commented Sep 9, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jgerigmeyer Sep 9, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LeaVerou commented Sep 9, 2022

jgerigmeyer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jgerigmeyer commented Sep 16, 2022

jgerigmeyer commented Sep 28, 2022

LeaVerou commented Sep 28, 2022

jgerigmeyer commented Sep 28, 2022

LeaVerou commented Sep 29, 2022 • edited Loading

jgerigmeyer commented Sep 30, 2022 • edited Loading

LeaVerou commented Sep 30, 2022

jgerigmeyer Sep 9, 2022 •

edited

Loading

LeaVerou commented Sep 29, 2022 •

edited

Loading

jgerigmeyer commented Sep 30, 2022 •

edited

Loading