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

jsmain:next should point to a “redistributable ES2015 bundle” #243

Closed
flying-sheep opened this issue Jan 8, 2016 · 15 comments
Closed

Comments

@flying-sheep
Copy link

what that means is “syntax the platform (browser/node) can read except for the ES2015 import/export syntax”

funnily my test project doesn’t work with specifically react-redux:

cc @Rich-Harris @TrySound @Victorystick

@flying-sheep
Copy link
Author

lol, currently it’s exactly the opposite: ES2015 with some ES2016 features except for ES2015 module syntax 😄

@gaearon
Copy link
Contributor

gaearon commented Jan 15, 2016

Please feel free to make a PR to fix this.

@flying-sheep
Copy link
Author

sure. first i need to know: are you comfortable switching to ES2015 module syntax and relying on babel to convert it to require calls?

in any case, the normal NPM bundle would be created as it currently is: babel with your current plugins and presets.

switching module syntax would mean for the redistributable bundle: current stuff minus module-transformer

not switching would mean: current stuff, after that use this to convert require syntax to ES2015 syntax (would probably need quite a bit of code to make it work outside of rollup.

@gaearon
Copy link
Contributor

gaearon commented Jan 15, 2016

sure. first i need to know: are you comfortable switching to ES2015 module syntax and relying on babel to convert it to require calls?

We just had a regression that forced us to go back to CommonJS: #233. I'm happy to revert this in React Redux 5 where we'll drop IE8 compat though.

@flying-sheep
Copy link
Author

hmm, was es3ify no option?

@gaearon
Copy link
Contributor

gaearon commented Jan 15, 2016

Seems inconvenient to add Browserify transforms to Webpack build.
And we don't want to support IE8 forever anyway. React is dropping it in next version, and so do we.

@flying-sheep
Copy link
Author

it’s also available for webpack but yeah, i can wait 😄

@flying-sheep
Copy link
Author

hmm, just thinking: your build system is basically npm run scripts right?

but using the babel CLI means i can’t programmatically do:

var options  = JSON.parse(require('fs').readFileSync('.babelrc', 'utf8'))
options.babelrc = false
options.splice(options.indexOf('es2015-loose'), 1, 'es2015-loose-rollup')
babelDir('src', options)

@thisconnect
Copy link

Would you accept a PR in the way redux has a rollup-able es6 module?

https://github.com/reactjs/redux/blob/master/package.json#L25

@gaearon
Copy link
Contributor

gaearon commented Apr 12, 2016

Yes.

@thisconnect
Copy link

Building ./es the redux way was straightforward and did build. I use npm link to test my local react-redux es build in a rollup test. When trying to rollup this error is thrown

[Error: Module node_modules/react-redux/node_modules/react/react.js does not export Component (imported by node_modules/react-redux/es/components/Provider.js)]

My understanding is that import { Component, PropTypes, Children } from 'react' does not work in this case. React does not provide es modules.

The following changes would fix that, but I do not really feel confy changing src for this. :/

-import { Component, PropTypes, Children } from 'react'
+import React from 'react'
 import storeShape from '../utils/storeShape'
 import warning from '../utils/warning'

+const { Component, PropTypes, Children } = React

At this point I almost want to give up rolling up dependencies and only rollup my own code. As the journey would countinue with react-router.

Interestingly react-router already distributes es modules and uses import React from 'react' instead import {something} from 'react', almost everywhere. I only found https://github.com/reactjs/react-router/blob/master/modules/PropTypes.js#L1 where it destructs directly from 'react'

remix-run/react-router#2530

@gaearon
Copy link
Contributor

gaearon commented Apr 16, 2016

Even if it worked, we don’t want to end up with the whole React in React Redux bundle. It’s not supposed to be embedded in the output. Webpack lets use set “externals” to express that; I presume Rollup has a similar mechanism? cc @Rich-Harris

@Rich-Harris
Copy link

Yep, just add external: [ 'react' ] to your Rollup config

@thisconnect
Copy link

Using babel directly (as it is done in redux) wont include react and keeps import React from 'react'

@timdorr
Copy link
Member

timdorr commented Oct 5, 2016

This should be fixed on next, I believe. Give 5.0.0-beta.2 a whirl. There is both a jsnext:main and module entry in the package.json now.

@timdorr timdorr closed this as completed Oct 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants