source icon indicating copy to clipboard operation
source copied to clipboard

Browser/Node.js Targets

Open JamieB-gu opened this issue 5 years ago • 5 comments

Is your feature request related to a problem? Please describe.

We're targeting approximately three different environments (in the editorial stream):

  • Node (10 for DCR, 12 for AR)
  • Dotcom client-side (Safari, Firefox, Chrome, Edge, Internet Explorer etc.)
  • Apps client-side (recent Safari and Chrome - webviews)

The support requirements for apps and dotcom on the client in particular are quite different. We have to transpile down to the level of IE support for dotcom, but will that result in shipping redundant polyfills to the apps?

Describe the solution you'd like

A definition of browserslist in package.json would be a simple solution to cover the lowest-common-denominator (i.e. the oldest Node/browsers) that we support. This would apply to all package builds in the editorial section of the design system

Describe alternatives you've considered

It would be nice if we could more precisely target the different projects to avoid shipping unnecessary polyfills. Perhaps it would be best to have the design system target a much more recent version of Node/browser-JavaScript, and leave the transpilation and polyfilling for older versions to each project.

Additional context

Derived from this discussion.

cc @SiAdcock @gtrufitt

JamieB-gu avatar Apr 02 '20 11:04 JamieB-gu

Note that on DCR we're actually generating two bundles. One is what you describe, the other is using https://github.com/babel/preset-modules which will bundle for browsers that support type="module" and therefore reduce the inline polyfilling.

Perhaps it would be best to have the design system target a much more recent version of Node/browser-JavaScript, and leave the transpilation and polyfilling for older versions to each project.

I don't know the setup for this but that would be my preference - does ESM allow us to do this?

cc/ @oliverlloyd @liywjl

gtrufitt avatar Apr 02 '20 11:04 gtrufitt

We have to transpile down to the level of IE support for dotcom, but will that result in shipping redundant polyfills to the apps?

I empathise with this reasoning. Would it be possible to guage the impact of shipping agressively transpiled code to apps? I wonder how much extra code is generated by supporting IE11. @gtrufitt in DCR, what is the difference in terms of % of code when comparing the preset-modules bundle with the preset-env bundle?

But 💯 if we can leave the more aggressive transpilation up to the consumer application, that would be my preference too. We'd have to help devs out with the config for this.

As for polyfilling, I'm currently against including polyfills in the dist code for src-* modules. Some of our platforms bring their own polyfills (i.e. using polyfill.io), so bundling them would add unnecessary bloat. For now, I'm cautiously using JS built-in methods that would work in IE11 and avoiding fancy newer stuff like Object.fromEntries that would require a polyfill. TypeScript will catch us out if we try to use newer methods like this.

This policy might change in the future, depending on whether we find newer methods introduced in ES2015+ useful. I'd love to get your opinions on this.

SiAdcock avatar Apr 03 '20 09:04 SiAdcock

Update on this

We've had some initial discussion in Client Side Infra and I think it'll be useful to bring up at our kickoff meeting. My assumptions going in:

  • we'd like to standardise our browserslist configs for consumable projects like Source and *-rendering packages
  • consumable projects would expose a CJS (main) build for older browsers and Node.js, and an ESM (module) build for modern browsers,. The former would be more aggressively transpiled than the latter. An example setup would use @babel/preset-env for the main build and @babel/preset-modules for the module build.
  • we'd leave it up to consuming applications to transpile either of these builds more aggressively if necessary

For now, I've added some quick cross-browser testing guidance to our docs. I will at least ensure all components and foundations work with the browsers and Node.js versions I've listed here as part of our acceptance criteria. The browsers are the current defaults for browserslist.

As an addendum, I tested the following browserslist config to see what would happen:

defaults
maintained node versions

It didn't seem to impact any browsers and but knocked ~3KB off each bundle for the Button component, and I'm not sure if that's awesome or scary!

SiAdcock avatar Jun 04 '20 15:06 SiAdcock

what is the difference in terms of % of code when comparing the preset-modules bundle with the preset-env bundle?

You can see this as it's tracked on any PR https://github.com/guardian/dotcom-rendering/pull/1542#issuecomment-638880237

I don't know the total total, but a couple of kb on our main bundle.

~3KB off each bundle for the Button component

Seems a lot! I wonder what modern code is so heavily used?!

gtrufitt avatar Jun 04 '20 15:06 gtrufitt

You can see this as it's tracked on any PR

This is really useful, thanks!

The above config knocks 3KB off each bundle, both modern and legacy. I'm not sure why this is happening. I expected the defaults not to include any Node.js versions, so adding maintained node versions would make the bundles heavier. I'm probably doing something wrong.

SiAdcock avatar Jun 04 '20 15:06 SiAdcock