d3 / d3-shape

Graphical primitives for visualization, such as lines and areas.
https://d3js.org/d3-shape
ISC License
2.47k stars 305 forks source link

Transpile source to support older browsers #137

Closed dagstuan closed 5 years ago

dagstuan commented 5 years ago

This PR adds transpiling of the source code when building with rollup to support older browsers. This allows for writing ES6 code such as object spread and for ( ... of ... ) while still supporting older browsers such as IE 11. Earlier the module field in package.json pointed directly at src/index.js. Since developers normally configure tools such as rollup or webpack to ignore the node_modules-folder, this meant that dependents of this package ended up with untranspiled code in their bundles if they intended to support browsers that do not support ES6 syntax. This PR builds a d3-shape.esm.js-file and uses that in the module-field instead. The d3-shape.esm.js-file contains transpiled code using @babel/preset-env except the ES5-style imports are preserved. This is what is recommended by rollup documentation: https://github.com/rollup/rollup/wiki/pkg.module#wait-it-just-means-import-and-export--not-other-future-javascript-features

dagstuan commented 5 years ago

Btw, I noticed that none of the d3-* packages actually transpile the source code, so a similar change should probably be done everywhere if this PR is accepted, I just started somewhere :)

mbostock commented 5 years ago

There’s already a UMD for old browsers.

I am planning on adding a bundled ES module to all the the D3 modules, but only by running the code through Rollup (as I’m already doing, just changing the output format to ES module rather than UMD).

dagstuan commented 5 years ago

Yes there's UMD, but it's not transpiled. So using ES6 would yield code that breaks when bundled unless consumers manually un-ignore the d3 packages from their webpack/rollout configs. Babel recommends ignoring node_modules when using babel-loader with webpack for example. https://github.com/babel/babel-loader#usage

dagstuan commented 5 years ago

Other popular libraries that use ES6 syntax (redux for example) transpile the code delivered in the main and module fields of their package so consumers can keep ignoring node_modules when building with webpack.

mbostock commented 5 years ago

The D3 modules don’t use ES6, though; they only use ES modules, and so no further transpilation is necessary beyond Rollup. I am planning on switching to ES6+ in the near future with a major version bump (as I’ve already done with d3-array@2), but I’m not going to transpile ES6+ back down to ES5 to support older environments; I’m only going to support ES6+ environments in the next major release of D3, as I plan on using ES6 runtime features and not just syntax.

curran commented 5 years ago

I've had great experiences with Buble as a lighter weight alternative to Babel.

dagstuan commented 5 years ago

@mbostock I think what you're proposing is a bad idea still in 2018. While I agree that ideally one should not have to transpile code for backwards compatibility, it's still an issue in 2018. Highlighted both by babel here: https://babeljs.io/blog/2018/06/26/on-consuming-and-publishing-es2015+-packages and by Dan Abramov on twitter here: https://twitter.com/dan_abramov/status/1009179550134296577

In summary, main and module fields should still serve transpiled code to consumers for backward compatibility, while some other (yet) unspecified key in package.json should be reserved for shipping es6 code in the future.

I can submit a separate PR to d3-array since that is the only library currently using new syntax, but I still think this change should be made to all the packages intending to switch to ES6 syntax.

torgeir commented 5 years ago

There’s already a UMD for old browsers.

Moving to es2015 features, like generators in d3-array@2, the UMD bundle is no longer for old browsers as the provided source has syntax they do not support.

Sticking with es2015 modules for a next major is a fair approach moving forward, however we could be even more helpful by explicitly stating the use of es2015 features, and that from these versions on d3 does no longer support environments that it previously has (e.g. like IE11 in @dagstuan's case), even with the UMD bundle, without users actually creating their own bundles (like the current main repo docs suggest), transpiling these features away for backwards compatibility.

Without it such a change might very well surprise people upgrading, as they would need to go through hoops actually compiling their dependencies, against recommendations by popular transpilers, if they still wish to serve they same set of browsers they previously have.

mbostock commented 5 years ago

The adoption of ES2015 features will be a major version change, so you’ll only get these features if you upgrade to the next major version when it’s released. If you don’t want these features, then you should stick with the current major version, which does not use these features and does not require transpilation (beyond the current Rollup). I’m not going to support transpiling ES2015 features for older browsers, so if you need to support those, you should not upgrade.

dagstuan commented 5 years ago

So what happens if I depend on a package that depends on d3? Not transpiling your code quickly leads to having to unignore all of node_modules when bundling, which means slow local transpilation for all consumers. What is the argument for not transpiling the code and helping the consumers of d3?

material-components had the same issue a few months back, which ended up with them delivering transpiled code: https://github.com/material-components/material-components-web-react/issues/107

mbostock commented 5 years ago

If that package decides to upgrade to the latest major version of D3 and adopt ES2015 features, then it’ll be in the same situation as the latest major version of D3: either its consumers will only support modern browsers, or they will need to transpile the code to whatever browsers they wish to support. If that package wishes to transpile its code to support older browsers, that’s fine, but I’m not taking on that additional responsibility. D3 is not a compatibility layer.