Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add build option for es modules #63

Open
nathancahill opened this issue Mar 28, 2017 · 11 comments
Open

Add build option for es modules #63

nathancahill opened this issue Mar 28, 2017 · 11 comments

Comments

@nathancahill
Copy link

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
Copy link
Owner

gajus commented Mar 28, 2017

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.

@gajus gajus added the question label Mar 28, 2017
@Velenir
Copy link
Contributor

Velenir commented Mar 28, 2017

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

@gajus
Copy link
Owner

gajus commented Mar 28, 2017

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

@gajus gajus closed this as completed Mar 28, 2017
@Velenir
Copy link
Contributor

Velenir commented Mar 28, 2017

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

@nathancahill
Copy link
Author

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
Copy link
Owner

gajus commented Mar 28, 2017

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
Copy link
Author

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
Copy link
Owner

gajus commented Mar 28, 2017

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

@nathancahill
Copy link
Author

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: webpack/webpack#1979

@nathancahill
Copy link
Author

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

@gajus gajus reopened this Mar 31, 2017
@gajus
Copy link
Owner

gajus commented Mar 31, 2017

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.

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

No branches or pull requests

3 participants