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

Yarn Plug'n'Play support for babel-node #8967

Open
mischnic opened this Issue Nov 3, 2018 · 9 comments

Comments

Projects
None yet
4 participants
@mischnic

mischnic commented Nov 3, 2018

Bug Report / Feature request

Current Behavior
Running babel-node with this package.json installed with yarn gives runtime errors.

/Users/niklas/Library/Caches/Yarn/v3/npm-@babel-node-7.0.0-20e55bb0e015700a0f6ff281c712de7619ad56f4/node_modules/@babel/node/lib/babel-node.js:99
    if (err.code !== "MODULE_NOT_FOUND") throw err;
                                         ^

Error: Package "@babel/node@7.0.0" (via "/Users/niklas/Library/Caches/Yarn/v3/npm-@babel-node-7.0.0-20e55bb0e015700a0f6ff281c712de7619ad56f4/node_modules/@babel/node/lib/babel-node.js") is trying to require the package "kexec" (via "kexec") without it being listed in its dependencies (@babel/core, @babel/polyfill, @babel/register, commander, fs-readdir-recursive, lodash, output-file-sync, v8flags, @babel/node)
    at makeError (.../.pnp.js:54:17)
    at Object.resolveToUnqualified (.../.pnp.js:3860:17)
    at Object.resolveRequest (.../.pnp.js:3931:31)
    at Function.Module._resolveFilename (.../.pnp.js:4111:30)
    at Function.Module._load (.../.pnp.js:4029:31)
    at Module.require (internal/modules/cjs/loader.js:643:17)
    at require (internal/modules/cjs/helpers.js:22:18)
    at /Users/niklas/Library/Caches/Yarn/v3/npm-@babel-node-7.0.0-20e55bb0e015700a0f6ff281c712de7619ad56f4/node_modules/@babel/node/lib/babel-node.js:94:19
    at /Users/niklas/Library/Caches/Yarn/v3/npm-v8flags-3.1.1-42259a1461c08397e37fe1d4f1cfb59cad85a053/node_modules/v8flags/index.js:131:14
    at /Users/niklas/Library/Caches/Yarn/v3/npm-v8flags-3.1.1-42259a1461c08397e37fe1d4f1cfb59cad85a053/node_modules/v8flags/index.js:39:14

Fixing this line to also ignore the error code UNDECLARED_DEPENDENCY

if (err.code !== "MODULE_NOT_FOUND") throw err;

leads to this error:

internal/modules/cjs/loader.js:589
    throw err;
    ^

Error: Cannot find module '@babel/polyfill'
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:587:15)
    at Function.Module._load (internal/modules/cjs/loader.js:513:25)
    at Module.require (internal/modules/cjs/loader.js:643:17)
    at require (internal/modules/cjs/helpers.js:22:18)
    at Object.<anonymous> (/Users/niklas/Library/Caches/Yarn/v3/npm-@babel-node-7.0.0-20e55bb0e015700a0f6ff281c712de7619ad56f4/node_modules/@babel/node/lib/_babel-node.js:73:1)
    at Module._compile (internal/modules/cjs/loader.js:707:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:718:10)
    at Module.load (internal/modules/cjs/loader.js:605:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:544:12)
    at Function.Module._load (internal/modules/cjs/loader.js:536:3)
error Command failed with exit code 1.

Expected behavior/code
No errors

Babel Configuration (.babelrc, package.json, cli command)

{
  "private": true,
  "devDependencies": {
    "@babel/core": "^7.0.0",
    "@babel/node": "^7.0.0",
    "@babel/preset-env": "^7.0.0",
    "@babel/preset-react": "^7.0.0"
  },
  "installConfig": {
    "pnp": true
  }
}
{
	"presets": [["env", {"target": "current"}], "react"]
}

Environment

  • Babel version(s): @babel/core: 7.1.2
  • Node version: v11.0.0
  • Yarn: 1.12.1
  • OS: macOS 10.13.6
  • Monorepo: no
  • How you are using Babel: babel-node

@arcanis might be interested

@babel-bot

This comment has been minimized.

Collaborator

babel-bot commented Nov 3, 2018

Hey @mischnic! We really appreciate you taking the time to report an issue. The collaborators
on this project attempt to help as many people as possible, but we're a limited number of volunteers,
so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack
community that typically always has someone willing to help. You can sign-up here
for an invite.

@xtuc xtuc added the area: node label Nov 3, 2018

@xtuc

This comment has been minimized.

Member

xtuc commented Nov 3, 2018

I haven't got the time to look into Yarn Plug'n'Play yet, what should babel/node do to make it work? Why is that dependency missing?

Also, could you please share your package.json?

@mischnic

This comment has been minimized.

mischnic commented Nov 3, 2018

What should babel/node do to make it work? Why is that dependency missing?

The main issue is that with PnP, requiring a package that is not explicitly listed in the dependencies (but still installed) throws an error (explained here).

Also, could you please share your package.json?

Here is the repo: https://github.com/mischnic/example-proton-pnp

EDIT: I've simplified the example

@arcanis

This comment has been minimized.

Contributor

arcanis commented Nov 3, 2018

Hey! The Module not found error at the end is typically thrown when the hook is not properly setup, which most often occurs when you spawn a node subprocess without forwarding process.execArgv.

I've looked a bit in @babel/node and there's one place where it happens - fixing it should get you past this message 😃

@mischnic

This comment has been minimized.

mischnic commented Nov 3, 2018

Changing

if (err.code !== "MODULE_NOT_FOUND") throw err;
const child_process = require("child_process");
const proc = child_process.spawn(process.argv[0], args, {
stdio: "inherit",
});

to

    if (err.code !== "MODULE_NOT_FOUND" && err.code !== "UNDECLARED_DEPENDENCY") throw err;

    const child_process = require("child_process");

    const proc = child_process.fork(args[0], args.slice(1), {
      stdio: "inherit"
    });

gives this new error:

~/Library/Caches/Yarn/v3/npm-@babel-core-7.1.2-f8d2a9ceb6832887329a7b60f9d035791400ba4e/node_modules/@babel/core/lib/config/files/plugins.js:152
    throw e;
    ^

Error: Cannot find module '@babel/preset-env' from '.../example-proton-pnp-babel'
    at Function.module.exports [as sync] (~/Library/Caches/Yarn/v3/npm-resolve-1.8.1-82f1ec19a423ac1fbd080b0bab06ba36e84a7a26/node_modules/resolve/lib/sync.js:43:15)
    at resolveStandardizedName (~/Library/Caches/Yarn/v3/npm-@babel-core-7.1.2-f8d2a9ceb6832887329a7b60f9d035791400ba4e/node_modules/@babel/core/lib/config/files/plugins.js:101:31)
    at resolvePreset (~/Library/Caches/Yarn/v3/npm-@babel-core-7.1.2-f8d2a9ceb6832887329a7b60f9d035791400ba4e/node_modules/@babel/core/lib/config/files/plugins.js:58:10)
    at loadPreset (~/Library/Caches/Yarn/v3/npm-@babel-core-7.1.2-f8d2a9ceb6832887329a7b60f9d035791400ba4e/node_modules/@babel/core/lib/config/files/plugins.js:77:20)
    at createDescriptor (~/Library/Caches/Yarn/v3/npm-@babel-core-7.1.2-f8d2a9ceb6832887329a7b60f9d035791400ba4e/node_modules/@babel/core/lib/config/config-descriptors.js:154:9)
    at items.map (~/Library/Caches/Yarn/v3/npm-@babel-core-7.1.2-f8d2a9ceb6832887329a7b60f9d035791400ba4e/node_modules/@babel/core/lib/config/config-descriptors.js:109:50)
    at Array.map (<anonymous>)
    at createDescriptors (~/Library/Caches/Yarn/v3/npm-@babel-core-7.1.2-f8d2a9ceb6832887329a7b60f9d035791400ba4e/node_modules/@babel/core/lib/config/config-descriptors.js:109:29)
    at createPresetDescriptors (~/Library/Caches/Yarn/v3/npm-@babel-core-7.1.2-f8d2a9ceb6832887329a7b60f9d035791400ba4e/node_modules/@babel/core/lib/config/config-descriptors.js:101:10)
    at presets (~/Library/Caches/Yarn/v3/npm-@babel-core-7.1.2-f8d2a9ceb6832887329a7b60f9d035791400ba4e/node_modules/@babel/core/lib/config/config-descriptors.js:47:19)

So a resolve.sync call here:

return resolve.sync(standardizedName, { basedir: dirname });

@mischnic

This comment has been minimized.

mischnic commented Nov 8, 2018

@arcanis If I change the .pnp.js to also shim the resolve npm package for babel/core, it works (and with the fork patch for babel above)!

Is there a reason to not always shim resolve? (I guess you want to fix resolve itself)

patchedModules.set('resolve', realResolve => {
 const mustBeShimmed = caller => {
   const callerLocator = exports.findPackageLocator(caller);
 
-  return callerLocator && callerLocator.name === 'liftoff';
+  return callerLocator && (callerLocator.name === 'liftoff' || "@babel/core");
 };
@mischnic

This comment has been minimized.

mischnic commented Nov 8, 2018

Why does babel-node try to require kexec ? When does that even succeed if it's not listed as a dependency?

@arcanis

This comment has been minimized.

Contributor

arcanis commented Nov 8, 2018

Hey! As you might have seen, I've reworked a bit the way the PnP hook is injected, and as a result forwarding the execArgv array isn't required anymore (still a good practice, maybe?). This only leaves the problem of kexec.

When does that even succeed if it's not listed as a dependency?

It succeeds accidentally when the hoisting puts kexec to the top-level, or when the user of babel-node depends on kexec. In essence, I think the idea is to make it an optional dependency (which is a use case I'm working to solve here: yarnpkg/rfcs#105).

Additionally, it doesn't break when the package isn't found because the require is done in a try/catch, but it also check for err.code being equal to MODULE_NOT_FOUND, which isn't the case on PnP (we throw semantic errors, so depending on the reason why the module isn't found it can be a different code such as UNDECLARED_DEPENDENCY, MISSING_PEER_DEPENDENCY, or MISSING_DEPENDENCY).

Is there a reason to not always shim resolve? (I guess you want to fix resolve itself)

Yup, that's the idea! I have a PR setup (browserify/resolve#170) to expose a better handling of this semantic case. It would still require babel-node to use the useProcessResolution flag, but that seems acceptable.

Now you mention it, maybe it should be configurable through the yarnrc files 🤔

That said, there's an easy workaround: just replace your .babelrc by .babelrc.js and use require.resolve. For example:

module.exports = {
  presets: [
    require.resolve(`@babel/preset-env`)
  ]
};

This is totally portable 🙂

@mischnic

This comment has been minimized.

mischnic commented Nov 8, 2018

forwarding the execArgv array isn't required anymore (still a good practice, maybe?).

Great!

When does that even succeed if it's not listed as a dependency?

It succeeds accidentally when the hoisting puts kexec to the top-level, or when the user of babel-node depends on kexec.

Let me rephrase my question: What's the intention here? Was kexec once a dependency and it was accidentally removed? Seems strange that it needs to be depend on by the user (and without hoisting that wouldn't even work)

In essence, I think the idea is to make kexec an optional dependency

But I think kexec would need be updated because it doesn't compile on Windows and is missing a os key in the package.json.

It would still require babel-node to use the useProcessResolution flag, but that seems acceptable.

Yes, in babel-core (but there are many calls to resolve):
https://github.com/babel/babel/blob/master/packages/babel-core/src/config/files/plugins.js#L99

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment