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

Please don’t publish experimental syntax on npm #107

Closed
gaearon opened this issue Jun 19, 2018 · 8 comments
Closed

Please don’t publish experimental syntax on npm #107

gaearon opened this issue Jun 19, 2018 · 8 comments

Comments

@gaearon
Copy link

gaearon commented Jun 19, 2018

Hi, I help maintain React and some other projects like Create React App. 👋

I recently saw in this issue (facebook/create-react-app#4648 (comment)) that this project uses experimental syntax (such as class properties) as part of the npm entry point.

I strongly encourage you to reconsider this decision. If the class properties transform ends up changing (which is very likely) or abandoned (not as likely, but could happen), having uncompiled packages on npm using it will create a huge amount of churn for everyone (including maintainers of this project). Not to mention this makes the project unusable in any environment that respects the spec (such as Node.js).

Please compile any experimental syntax away before publishing. If you’re convinced that keeping import uncompiled is worth it (which is a whole separate can of worms as you can see in graphql/graphql-js#1248), please compile at least the non-standard syntax away when publishing. That includes JSX too.

I understand this might seem like an inconvenience now. But it will be much better to do now than deal with pain for months when build tools and the spec changes. Thanks.

@gaearon
Copy link
Author

gaearon commented Jun 19, 2018

To be clear, it's totally possible with Babel to keep the import/export in the bundle (if you consider it worth the Node compatibility issues until mjs shakes out), but compile the rest of the syntax. Many React projects already do that, and I'd be happy to help in case you have any issues with this approach.

@moog16
Copy link
Contributor

moog16 commented Jun 20, 2018

@gaearon you are suggesting that we switch our npm entry point to transpiled ES5 syntax? Kind of like what this issue is talking about. We do provide a non experimental syntax version as mentioned in the issue.

@gaearon
Copy link
Author

gaearon commented Jun 20, 2018

I think that would be a great first step!

But my understanding is that you still want to provide a source version so I'd love to learn why. If it's just for better tree shaking with e.g. webpack, then you could provide a version with import/export (but still no experimental syntax) instead.

@gaearon
Copy link
Author

gaearon commented Jun 20, 2018

In particular, my concern is that publishing the source code (even as an optional other entry point) will encourage people to use it (e.g. because they think it will make their app smaller) and cause issues for the ecosystem when it starts switching to incompatible tooling. For example, even Babel 6 and Babel 7 handle class properties differently by default. And the spec isn't even finalized yet.

@moog16
Copy link
Contributor

moog16 commented Jun 20, 2018

Yes that was our initial thought. I'll bring this to the team in the morning so that we're all on the same page to reach a consensus. But my thinking right now is at the very least updating the npm entry points for the packages.

@gaearon
Copy link
Author

gaearon commented Jun 20, 2018

Thanks! I'm always happy to chat about this, feel free to ping me anytime.

I care about this because I've seen people burned by this multiple times (e.g. Babel 5 -> Babel 6 transition, RN upgrades), and unfortunately the people who suffer the most are usually beginners who just followed a tutorial and can't fix anything in a third-party package.

Your fast response is very much appreciated!

@hzoo
Copy link

hzoo commented Jun 20, 2018

Happy to chat as well (slack/hangouts) if you need to talk through something too (I help maintain Babel).

@moog16
Copy link
Contributor

moog16 commented Jun 20, 2018

Thanks both for your input and support. <3 OS community. I opened PR #108, which should default to ES5 transpiled React Components.

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