Skip to content

Remove babel-standalone#6135

Closed
jridgewell wants to merge 1 commit intobabel:7.0from
jridgewell:revert-6029
Closed

Remove babel-standalone#6135
jridgewell wants to merge 1 commit intobabel:7.0from
jridgewell:revert-6029

Conversation

@jridgewell
Copy link
Copy Markdown
Member

This reverts #6029 due to issues in #6120.

We need to first move babel-loader into the monorepo (and update its
dependencies on babel-core). Afterwards, we can re-add
babel-standalone.

This reverts babel#6029 due to issues in babel#6120.

We need to first move `babel-loader` into the monorepo (and update its
dependencies on `babel-core`). Afterwards, we can re-add
`babel-standalone`.
@jridgewell jridgewell requested a review from Daniel15 August 19, 2017 04:20
@mention-bot
Copy link
Copy Markdown

@jridgewell, thanks for your PR! By analyzing the history of the files in this pull request, we identified @hzoo, @ChauTNguyen and @danez to be potential reviewers.

@Daniel15
Copy link
Copy Markdown
Member

Daniel15 commented Aug 19, 2017

Ahh, this is unfortunate :( Can we just comment out the build for now rather than completely deleting it? Just have it sit there until fixed.

Would it be easy to move babel-loader into the monorepo?

@jridgewell
Copy link
Copy Markdown
Member Author

Can we just comment out the build for now rather than completely deleting it?

We could do it that way, but is there a reason? It should be as simple as git revert #6135 to put this back in after we get babel-loader.

Would it be easy to move babel-loader into the monorepo?

That'd be a question for @danez.

@Daniel15
Copy link
Copy Markdown
Member

Daniel15 commented Aug 19, 2017

is there a reason?

Mainly thrash, and the fact that we want to move babili-standalone (now babel-minify-standalone) into the minify repo, and it'll reuse the shared Gulp tasks. We can put that on hold for now, but there's also some tooling that relies on the babel-standalone builds now (like the REPL previews) so if possible I'd rather roll forwards instead of back.

Is the issue just that babel-loader's Babel reference is not lenient enough? If so, can I simply make my own fork of babel-loader to fix it?

@Daniel15
Copy link
Copy Markdown
Member

Daniel15 commented Aug 19, 2017 via email

@jridgewell
Copy link
Copy Markdown
Member Author

we can use a custom Webpack resolver plugin that always prefers the monorepo version

If you can get that to work, then perfect.

@danez
Copy link
Copy Markdown
Member

danez commented Aug 19, 2017

Couldn't you simply set a resolve.alias in the webpack config to point to the correct babel-core instance? https://webpack.js.org/configuration/resolve/#resolve-alias

Would it be easy to move babel-loader into the monorepo?

I'm not a big fan of monorepos for everything, but it would of course be possible. Tests are in jest and not mocha, but other than that should be doable.
In my opinion integrations should not be part of the monorepo.

@Daniel15
Copy link
Copy Markdown
Member

Daniel15 commented Aug 19, 2017 via email

@Daniel15
Copy link
Copy Markdown
Member

Hmm, this is super tricky. It's not actually a Webpack issue, it's a Node.js module resolution problem. Here's the stack trace I'm seeing:

Module build failed: Error: [BABEL] C:\src\babel\packages\babel-standalone\src\index.js: You gave us a visitor for the node type "PrivateName" but it's not a valid type
    at verify (C:\src\babel\node_modules\babel-core\node_modules\babel-traverse\lib\visitors.js:156:13)
    at Function.explode (C:\src\babel\node_modules\babel-core\node_modules\babel-traverse\lib\visitors.js:59:3)
    at instantiatePlugin (C:\src\babel\node_modules\babel-core\lib\config\option-manager.js:304:27)
    at loadPluginDescriptor (C:\src\babel\node_modules\babel-core\lib\config\option-manager.js:280:14)
    at C:\src\babel\node_modules\babel-core\lib\config\option-manager.js:73:14
    at Array.map (native)
    at OptionManager.mergeOptions (C:\src\babel\node_modules\babel-core\lib\config\option-manager.js:72:34)
    at C:\src\babel\node_modules\babel-core\lib\config\option-manager.js:100:15
    at Array.forEach (native)
    at OptionManager.mergeOptions (C:\src\babel\node_modules\babel-core\lib\config\option-manager.js:99:15)
    at C:\src\babel\node_modules\babel-core\lib\config\option-manager.js:100:15
    at Array.forEach (native)
    at OptionManager.mergeOptions (C:\src\babel\node_modules\babel-core\lib\config\option-manager.js:99:15)
    at C:\src\babel\node_modules\babel-core\lib\config\option-manager.js:100:15
    at Array.forEach (native)
    at OptionManager.mergeOptions (C:\src\babel\node_modules\babel-core\lib\config\option-manager.js:99:15)
    at C:\src\babel\node_modules\babel-core\lib\config\option-manager.js:100:15
    at Array.forEach (native)
    at OptionManager.mergeOptions (C:\src\babel\node_modules\babel-core\lib\config\option-manager.js:99:15)
    at OptionManager.init (C:\src\babel\node_modules\babel-core\lib\config\option-manager.js:131:14)
    at manageOptions (C:\src\babel\node_modules\babel-core\lib\config\option-manager.js:59:30)
    at loadConfig (C:\src\babel\node_modules\babel-core\lib\config\index.js:20:38)
    at Object.transform (C:\src\babel\node_modules\babel-core\lib\transformation\pipeline.js:48:37)
    at transpile (C:\src\babel\node_modules\babel-loader\lib\index.js:49:20)
    at Object.module.exports (C:\src\babel\node_modules\babel-loader\lib\index.js:174:20)

node_modules\babel-core is version 7.0.0-alpha.18 from npm (as that's what the root package.json references), whereas the version in the repo is 7.0.0-alpha.19.

I think you were on the right track in #6132, with some refinement. If we can install babel-loader into the babel-standalone directory rather than the root, it will use babel-standalone's version of babel-core rather than the root version of babel-core, which should fix it (as babel-standalone references the monorepo's version of babel-core)

The other option is to override the Node.js module resolution algorithm (eg. by overriding Module._load) and force it to load the right babel-core version.

I'll try the easiest approach first and see how I go.

@Daniel15
Copy link
Copy Markdown
Member

#6137 should fix this. Thank you so much for the initial investigation, @jridgewell!

@Daniel15 Daniel15 closed this Aug 19, 2017
@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 6, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

outdated A closed issue/PR that is archived due to age. Recommended to make a new issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants