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

Closed
echenley opened this Issue Jan 5, 2017 · 9 comments

Comments

Projects
None yet
2 participants
@echenley

echenley commented Jan 5, 2017

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).

@echenley echenley changed the title from colors.es2015.js and colors.js have different API to colors.es2015.js and colors.js have different APIs Jan 5, 2017

@echenley echenley referenced this issue in casesandberg/react-color Jan 5, 2017

Closed

Broken material-colors API #318

@shuhei

This comment has been minimized.

Show comment
Hide comment
@shuhei

shuhei Jan 5, 2017

Owner

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.

Owner

shuhei commented Jan 5, 2017

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

This comment has been minimized.

Show comment
Hide comment
@echenley

echenley Jan 5, 2017

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 Jan 5, 2017

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

This comment has been minimized.

Show comment
Hide comment
@echenley

echenley Jan 5, 2017

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

echenley commented Jan 5, 2017

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

@shuhei

This comment has been minimized.

Show comment
Hide comment
@shuhei

shuhei Jan 5, 2017

Owner

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.

Owner

shuhei commented Jan 5, 2017

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

This comment has been minimized.

Show comment
Hide comment
@shuhei

shuhei Jan 5, 2017

Owner

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

Owner

shuhei commented Jan 5, 2017

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

@shuhei shuhei closed this in 15be66d Jan 5, 2017

@shuhei

This comment has been minimized.

Show comment
Hide comment
@shuhei

shuhei Jan 5, 2017

Owner

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

Owner

shuhei commented Jan 5, 2017

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

@echenley

This comment has been minimized.

Show comment
Hide comment
@echenley

echenley Jan 5, 2017

Awesome, thank you!

echenley commented Jan 5, 2017

Awesome, thank you!

@shuhei

This comment has been minimized.

Show comment
Hide comment
@shuhei

shuhei Jan 5, 2017

Owner

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
};
Owner

shuhei commented Jan 5, 2017

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

This comment has been minimized.

Show comment
Hide comment
@shuhei

shuhei Jan 5, 2017

Owner

Done in material-colors@1.2.5.

Owner

shuhei commented Jan 5, 2017

Done in material-colors@1.2.5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment