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

Fixes babel-register for Windows paths #10154

Closed
wants to merge 1 commit into from
Closed

Fixes babel-register for Windows paths #10154

wants to merge 1 commit into from

Conversation

rozele
Copy link
Contributor

@rozele rozele commented Sep 28, 2016

Babel was not compiling anything from react-packager/src when run from Windows, including the recently added node-haste stuff, which has been causing problems for some people. This change fixes that.

Fixes #10033
Fixes #8899

Babel was not compiling anything from react-packager/src when run from Windows, including the recently added node-haste stuff, which has been causing problems for some people. This change fixes that.

Fixes #10033
Fixes #8899
@facebook-github-bot
Copy link
Contributor

By analyzing the blame information on this pull request, we identified @janicduplessis and @sam-swarr to be potential reviewers.

@facebook-github-bot facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Sep 28, 2016
@rozele
Copy link
Contributor Author

rozele commented Sep 28, 2016

babel-core (used by babel-register) takes a dependency on slash, which only "normalizes" paths (with '/') if they consist of only ASCII characters. So, babel-register breaks down for any React Native users on Windows when their project paths contain non-ASCII characters.

@rozele
Copy link
Contributor Author

rozele commented Oct 3, 2016

This would be redundant if babel/babel#4631 gets accepted.

@rozele
Copy link
Contributor Author

rozele commented Oct 5, 2016

We may want to consider merging this, as I don't know when or if the change to babel will be accepted.

@rozele
Copy link
Contributor Author

rozele commented Oct 14, 2016

The babel PR is accepted, so once the next version of babel is released, we can close out this PR and instead upgrade babel.

@facebook-github-bot
Copy link
Contributor

It's been a while since the last commit was reviewed and the labels show this pull request needs review. Based on the blame information for the files in this pull request we identified @davidaurelio as a potential reviewer. Could you take a look please or cc someone with more context?

@hramos
Copy link
Contributor

hramos commented Nov 11, 2016

@rozele do you still plan to ship this?

@hramos
Copy link
Contributor

hramos commented Nov 11, 2016

cli.js conflict needs to be addressed.

@rozele
Copy link
Contributor Author

rozele commented Nov 14, 2016

Thanks for checking @hramos. Since my PR to babel was accepted, and someone recently upgraded babel (680df89), this PR is no longer relevant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Facebook Partner: Facebook Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

react-native cli tool syntax error on windows 10 Error in react-packager
3 participants