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

What's the verdict on mjs support? #4317

Closed
craigmulligan opened this issue Apr 17, 2018 · 11 comments
Closed

What's the verdict on mjs support? #4317

craigmulligan opened this issue Apr 17, 2018 · 11 comments
Milestone

Comments

@craigmulligan
Copy link
Contributor

Is this a bug report?

No, It's a question on the release conventions of the project. I was wondering how fixes for the current release are ported to the next branch?

I was depending on this bug fix https://github.com/facebook/create-react-app/pull/4085/files, from 1.1.2 but then needed some updated dependencies, so started using react-scripts@next but found that that fix is not present.

I'd expect all of master/current release to be merged into next is that not the case? or is it a manual process?

Thanks in advance! 👍

@craigmulligan craigmulligan changed the title Updating next branch with master? keeping next branch in sync with bug fixes from master Apr 17, 2018
@gaearon
Copy link
Contributor

gaearon commented Apr 17, 2018

I believe it was cherry picked to next but we haven’t cut a new next version yet. Plan to in a few days.

@craigmulligan
Copy link
Contributor Author

Okay, thanks for the clarification @gaearon will wait for the next next release 👌

@craigmulligan
Copy link
Contributor Author

craigmulligan commented Apr 17, 2018

Oh it looks like it hasn't been cherry-picked into next yet. I've opened a PR doing that #4318.

@Timer
Copy link
Contributor

Timer commented Apr 17, 2018

This was intentionally not cherry-picked because it's a sub-optimal bugfix.
Per comment on the PR's thread, I think @gaearon was in favor of removing mjs support all together (in 2.0) until we can support it properly (this is my position, anyway).

@Timer Timer added this to the 2.0.0 milestone Apr 17, 2018
@Timer Timer changed the title keeping next branch in sync with bug fixes from master What's the verdict on mjs support? Apr 17, 2018
@craigmulligan
Copy link
Contributor Author

@Timer oh thanks for pointing that out, read the full thread now and agree it's suboptimal. Shall I close the PR and keep the issue open so we can track the mjs plan?

@Timer
Copy link
Contributor

Timer commented Apr 18, 2018

You can leave the PR open; if we don't figure out proper support we'll probably roll with it.

Would you like to help with ensuring proper support? The Jest feature request would be a great start.

@iansu
Copy link
Contributor

iansu commented Apr 18, 2018

What exactly do we need for proper support? Is it just Jest and webpack handling .mjs files appropriately? If that's in place do we need to do anything more on our side?

@Timer
Copy link
Contributor

Timer commented Apr 18, 2018

Jest and then we need to make sure that webpack resolves starting with js or mjs properly from main when module is missing (which means we need to merge the PR removing the extension from our paths.appIndexJs or whatever).

@chadfurman
Copy link

3 months later -- where are with with this?

@chadfurman
Copy link

I see in the latest webpack.config.dev.js for react-scripts that we have .mjs before .js in the extensions list

@Timer
Copy link
Contributor

Timer commented Sep 24, 2018

We're dropping mjs support in 2.0 until Jest supports it, please follow jestjs/jest#4842.

@Timer Timer closed this as completed Sep 24, 2018
@lock lock bot locked and limited conversation to collaborators Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants