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

Question: Can babel-plugin-transform-runtime 's moduleName be removed? #1893

Closed
cheapsteak opened this issue Mar 25, 2017 · 4 comments
Closed

Comments

@cheapsteak
Copy link

cheapsteak commented Mar 25, 2017

moduleName: path.dirname(require.resolve('babel-runtime/package'))

This results in the full absolute path to be injected into the transpiled result, so you see

var _regenerator = require('/Users/USER/projects/PROJECT/node_modules/babel-runtime/regenerator');

instead of

var _regenerator = require('babel-runtime/regenerator');

As a workaround, I overwrote the plugin options by adding this to the end of my own babelrc's plugins array

["babel-plugin-transform-runtime",  {
      "helpers": false,
      "polyfill": false,
      "regenerator": true
    }]

I'm curious what problem the full path solved, and whether it's possible to solve that problem a different way? Tracing back the file history only lead me to the commit for the monorepo migration

Feel free to close the issue, I wasn't sure where else to ask this question

@Timer
Copy link
Contributor

Timer commented Mar 25, 2017

See #255, temporary fix #262 and permanent fix #535.

Node's resolution algorithm does not recursively search to any depth and strictly checks parent directories.
This is (or was) required due to the way we hide the babel dependency.

If you can show that it's no longer necessary, we can consider removing it. Just make sure it still works on NPM 2.x (Node 4).


May I ask why this is a problem for you?

This should be compiled away when generating a production build.

@cheapsteak
Copy link
Author

cheapsteak commented Mar 26, 2017

Ah that makes sense. Just tested and looks like it is still required for Node 4, I'll close the ticket since the question's answered :)

I see from the previous code comments that there was previously a notice in the babel config for ejected apps saying it's okay to remove that line post-ejection. Now that the babel config is hidden behind the react-app preset, it became a bit more difficult to find the cause. Not sure what a good solution to that might be though

@cheapsteak
Copy link
Author

cheapsteak commented Mar 26, 2017

I'm misusing the preset for a node app :p - building via babel's directory compilation, and used react-apps' babel presets out of laziness. Was tripped up a bit by the absolute path when I deployed the transpiled js and saw it was trying to require regenerator from a path on the computer where it was transpiled

@Timer
Copy link
Contributor

Timer commented Mar 26, 2017

That's not misuse! We encourage it.
But yes, this is still required then since NPM@2 doesn't hoist packages (and in 3 for preventing wrong version).
I suggest you bundle with something that takes care of this (like webpack), fork the package, or live with it. :D

@lock lock bot locked and limited conversation to collaborators Jan 22, 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

2 participants