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

Remove runtime alias hack #5142

Merged
merged 8 commits into from
Sep 27, 2018
Merged

Remove runtime alias hack #5142

merged 8 commits into from
Sep 27, 2018

Conversation

Timer
Copy link
Contributor

@Timer Timer commented Sep 27, 2018

Inspired by #5136, fixes #5116.

@Timer Timer added this to the 2.0.0 milestone Sep 27, 2018
// project
// https://github.com/babel/babel/blob/090c364a90fe73d36a30707fc612ce037bdbbb24/packages/babel-plugin-transform-runtime/src/index.js#L35-L42
absoluteRuntime: path.dirname(
require.resolve('@babel/runtime/package.json')
Copy link
Contributor

Choose a reason for hiding this comment

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

We can pass own path here btw I think. See usage in babel/babel#8435.

@gaearon
Copy link
Contributor

gaearon commented Sep 27, 2018

That's more or less what we used to do until I changed it in #2175. I was fixing #2172 (comment): it embedded my name in a library build. I think this would regress in the same way: any library using babel-preset-react-app would suddenly get hardcoded runtime path.

Maybe the solution is to make it an option, and then pass it through from webpack config.

Copy link
Contributor

@gaearon gaearon left a comment

Choose a reason for hiding this comment

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

We shouldn't break libraries

@gaearon
Copy link
Contributor

gaearon commented Sep 27, 2018

Ideally we should also try to create a relative path somehow. So that even in sourcemaps you'd see something like ../../../node_modules/@babel/runtime instead of your username etc.

Copy link
Contributor

@gaearon gaearon left a comment

Choose a reason for hiding this comment

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

Ideally with some e2e regression tests

@Timer Timer merged commit 27ac52a into facebook:master Sep 27, 2018
@Timer Timer deleted the fix-runtime branch September 27, 2018 20:53
@Timer Timer mentioned this pull request Sep 27, 2018
zmitry pushed a commit to zmitry/create-react-app that referenced this pull request Sep 30, 2018
* Remove runtime alias hack

* Pass absolute path to preset

* Change comment

* Give a relative path to absolute runtime

* Clean up config

* Tweak again

* Make absolute runtime the default

* Remove runtime package from error overlay
@lock lock bot locked and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

project can not work after upgrade react-scripts form 2.0.0-next.a671462c to 2.0.0
3 participants