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

Babel 6 and dep updates #63

Merged
merged 13 commits into from
Jun 21, 2016
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
"name": "react-formal",
"version": "0.17.9",
"description": "Classy HTML form management for React",
"jsnext:main": "src/index.js",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not correct – jsnext:main should be code transpiled to use ES modules rather than CJS modules, not the untranspiled source.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not following you. The source appears to me to be ES2015, and tools that acknowledge jsnext:main can consume it as part of their transpilation pipelines. So as far as my experience with rollup goes, I need to do nothing to the src and this will work fine. Can you point to my misunderstanding?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The source is not ES2015 – it's whatever is valid Babel stage 0.

Moreover the target for jsnext:main isn't ES2015 anyway – it's ES5 with ES modules (rather than CJS modules).

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the big thing here is that the source uses jsx, which isn't valid javascript at all in its uncompleted form

Copy link
Contributor Author

@rosskevin rosskevin Jun 20, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me restate then. jsnext:main isn't a standard, it's a pseudo and not one that is fully adopted. I don't think we want to bike shed it into jsnext:main:jsx or something like that in order to allow an app to use it. I see the point of wanting to constrain it to ES only, but what ES only app (have to assume it is react, though not necessarily jsx) would use this UI-only library in a transpilation pipeline that couldn't handle JSX? I realize this is a bigger and more complicated conversation, but lacking another meta indicator, why not use this until we have another way? As-is, I still have to go out of my way to include this in my webpack pipeline.

Copy link
Owner

@jquense jquense Jun 20, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are right we don't want bikeshedding anything, we don't need an indicator for general untranspiled code, the src folder signals enough.

JSNEXT is specifically for signally to environments and bundlers where the es module versions of your code is as an interminable solution until transpilation of modules isn't needed because they are natively supported. using it however we feel defeats the usefulness and validity of the convention ppl are trying to establish to solve a specific problem.

Copy link
Owner

@jquense jquense Jun 20, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure what you mean about going out of your way to include this with webpack though... are you saying you can't just require it? you shouldn't have to configure anything to use this.

Copy link
Contributor Author

@rosskevin rosskevin Jun 20, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, webpack doesn't look to resolve jsnext:main: webpack/webpack#1979 (comment)

I'm prototyping some local resolver code that will add these to my src loader pipeline, but it is really naive and only one level down. Without some kind of indicator, I'll be leaving out react-formal. Not a big deal, I'm just looking for a way to move forward.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not clear why you'd even need webpack to look at that it already understands main. The code is already compiled there is nothing else to be done. Eventually you can get things like tree shaking in webpack 2 but again the point is not to have a general entry point for uncompiled code, but as a way to say "this is where code is that's in the es module format"

Copy link
Contributor Author

@rosskevin rosskevin Jun 20, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm looking at tree shaking and general benefits of navigating code while developing. My local workaround failed miserably, so we are pretty much stuck until webpack catches up and some standardization happens anyway.

"main": "lib/index.js",
"author": {
"name": "Jason Quense",
Expand Down