shuhei / material-colors

Colors of Google's Material Design made available to coders
https://shuheikagawa.com/material-colors/
ISC License
270 stars 41 forks source link

colors.es2015.js and colors.js have different APIs #16

Closed echenley closed 7 years ago

echenley commented 7 years ago

In Babel, using export var is different than using module.exports = {}. Currently if using the es6 version you need to do import * as colors from 'material-colors' vs import colors from 'material-colors' for es5 (the latter is undefined since there's no default export).

shuhei commented 7 years ago

If the es module exports the entire JSON as default export, there is not benefit for tree shaking and no point for the es module. But the file is not so big. Probably we don't need jsnext:main or module for such a small library.

echenley commented 7 years ago

Yeah, colors.js and colors.es2015.js need to have the same API, otherwise it is going to break down in situations where a dependency expects one version and you expect the other.

For instance, we are using react-color, which uses the UMD version of material-colors when it builds. That means that react-color expects the output:

import colors from 'material-colors'
console.log(colors.red)

// transpiles (more-or-less) to
var colors = require('material-colors') // { default: { red: ...etc } }
console.log(colors.default.red)

However, since we are using webpack 2, this code tries to use colors.es2015.js and fails:

import colors from 'material-colors'
console.log(colors.red)

// transpiles (more-or-less) to
var colors = require('material-colors') // { red: ...etc }
console.log(colors.default.red) // can't read property 'red' of undefined

It would work for us if react-color changed:

import * as colors from 'material-colors'
console.log(colors.red)

But that would break when using webpack 1. Basically if you want to support both es6 and es5, you will need to ensure that your es5 and es6 have the same API. Maybe there's a grunt plugin you could use to generate a compatible umd bundle instead of templates? Otherwise like you said, you could choose not to publish an es6 version.

echenley commented 7 years ago

Sorry, I know this is the annoying state of javascript bundling. Thanks for the response!

shuhei commented 7 years ago

What does the UMD bundle plugin does for ES module?

Anyway, because CommonJS and ES Module have different interfaces, it is hard to have exactly same API. To achieve it, an ES Module has to export an object with all the exports as props like module.exports, but it misses the point of ES Module. No bundle size reduction for tree shaking.

The ES Module bundlers like Rollup and Webpack 2 works well only when all the intermediate dependencies, react-color in this case, support the tentative ES Module community standard, which will take some time. I will just remove jsnext:main and module.

shuhei commented 7 years ago

@echenley No problem! I know this is the way the JavaScript ecosystem has evolved, and it's a good learning opportunity for me 😃

shuhei commented 7 years ago

jsnext:main and module are removed in material-colors@1.2.4.

echenley commented 7 years ago

Awesome, thank you!

shuhei commented 7 years ago

Now I come up with a better idea. We could have a default export in addition to named exports. It's compatible to CommonJS API (thanks to Babel's interop) and also available for tree shaking.

export var red = { ... };
export var blue = { ... };
export default {
  red: red,
  blue: blue
};
shuhei commented 7 years ago

Done in material-colors@1.2.5.