search

gajus / redux-immutable

redux-immutable is used to create an equivalent function of Redux combineReducers that works with Immutable.js state.
Other
1.88k stars 82 forks source link

Add build option for es modules #63

Open nathancahill opened 7 years ago

nathancahill commented 7 years ago

Redux (and a lot of other React libs) have a ES2015 modules version of the build: https://github.com/reactjs/redux/blob/master/package.json#L24

This is really handy for tools like Rollup, since the "module" key in package.json can be pointed at ./es/index.js. I'm using a fork of redux-immutable with these builds and it works well.

Would you consider a PR that adds multiple build outputs to match how Redux is built?

gajus commented 7 years ago

Would you consider a PR that adds multiple build outputs to match how Redux is built?

Whats the benefit of that in the context of redux-immutable?

redux-immutable utilises all the files that it loads. Rollup cannot remove anything.

Velenir commented 7 years ago

Does rollup implement tree shaking? Then maybe it can remove something in production.

gajus commented 7 years ago

Then maybe it can remove something in production.

Remove what? Rollup is relevant only if it can determine (statically) that something is not being used. Tree shaking will be of value only to large libraries (think lodash). Purpose built libraries like redux-immutable have no benefit in using rollup (unless they themselves have big dependencies).

Velenir commented 7 years ago

You're right. At best it could get rid of utilities/getStateName.

nathancahill commented 7 years ago

The benefit of including a "module" build in Redux-Immutable is that Rollup only looks at import/export statements to determine which dependencies to include. In this case, if Immutable were globally available, it would still be included in a bundle that uses Redux-Immutable, because Rollup doesn't understand the require('immutable') statement. It also speeds up the build process, since ES2015 modules can be included directly by Rollup without transpiling.

gajus commented 7 years ago

I am all pro-rollup approach, I am just not happy with the-Rollup. I think the user base is too insignificant to cater for. This will land in one shape or form in Node.js v8 and webpack v3.

nathancahill commented 7 years ago

What I'm proposing isn't specific to Rollup. It's good practice for any library. Especially since it would match the rest of the React ecosystem.

gajus commented 7 years ago

jsnext:main package.json field is rollup specific. Of course, correct me if I am wrong.

nathancahill commented 7 years ago

jsnext:main is now superseded by the module key, which is used to specify the es2015 entrypoint. Although Rollup proposed the original idea, Webpack 2 now supports the standard. This is the direction libraries are going. See the long discussion here: https://github.com/webpack/webpack/issues/1979

nathancahill commented 7 years ago

So before I publish a fork on npm, that's a hard no on adding es modules to your build?

gajus commented 7 years ago

So before I publish a fork on npm, that's a hard no on adding es modules to your build?

Go ahead with a fork.

I will need to read up about the module situation in Node.js land today and whats the consensus and long term strategy. This is not going to happen today or tomorrow.