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

[Bug] nyc fails with PnP on Windows #1841

Closed
1 task
clemyan opened this issue Sep 17, 2020 · 4 comments
Closed
1 task

[Bug] nyc fails with PnP on Windows #1841

clemyan opened this issue Sep 17, 2020 · 4 comments
Labels
bug Something isn't working external bug This issue highlights a bug in another project unreproducible This issue cannot be reproduced on master

Comments

@clemyan
Copy link
Member

clemyan commented Sep 17, 2020

  • I'd be willing to implement a fix

Describe the bug

When using PnP on Windows, running nyc fails with the following error:

C:\path\to\project\.pnp.js:6338
    throw firstError;
    ^

Error: Your application tried to access node-preload.js, but it isn't declared in your dependencies; this makes the require call ambiguous and unsound.

Required package: node-preload.js (via "node-preload.js")
Required by: C:\path\to\project\

Require stack:
- internal/preload
    at internalTools_makeError (C:\path\to\project\.pnp.js:6082:34)
    at resolveToUnqualified (C:\path\to\project\.pnp.js:7034:23)
    at resolveRequest (C:\path\to\project\.pnp.js:7132:29)
    at Object.resolveRequest (C:\path\to\project\.pnp.js:7198:26)
    at Function.external_module_.Module._resolveFilename (C:\path\to\project\.pnp.js:6315:34)
    at Function.external_module_.Module._load (C:\path\to\project\.pnp.js:6180:48)
    at Module.require (internal/modules/cjs/loader.js:1089:19)
    at Module._preloadModules (internal/modules/cjs/loader.js:1345:12)
    at loadPreloadModules (internal/bootstrap/pre_execution.js:439:5)
    at prepareMainThreadExecution (internal/bootstrap/pre_execution.js:71:3)

This does not happen and nyc correctly collects coverage when:

  • using the node-modules linker
  • running nyc inside a docker container using WSL2 backend
  • running nyc on a Linux (specifically CentOS 7) VM

To Reproduce

The minimal information needed to reproduce your issue (ideally a package.json with a single dep). Note that bugs without minimal reproductions might be closed.

IMPORTANT: We strongly prefer reproductions that use Sherlock. Please check our documentation for more information: https://yarnpkg.com/advanced/sherlock

Reproduction
const { promises: { writeFile } } = require('fs')

await packageJsonAndInstall({
    devDependencies: {
        nyc: '^15.1.0'
    }
})
await writeFile('test.js', 'console.log("test")')

await expect(yarn('nyc', 'node', 'test.js')).resolves.toBeTruthy()

Environment if relevant (please complete the following information):

  • OS: Windows 10 Home Build 19041.508
  • Node version: 14.11.0, 12.18.4
  • Yarn version: 2.2.2, 2.2.2-git.20200914.bc3c466e

Additional context

istanbuljs/nyc#1308 probably relevant? However, unlike pnpm, running yarn nyc --use-spawn-wrap (Windows, PnP) will cause nyc to not run the given test command at all and collects no coverage.

@clemyan clemyan added the bug Something isn't working label Sep 17, 2020
@yarnbot yarnbot added the unreproducible This issue cannot be reproduced on master label Sep 17, 2020
@yarnbot

This comment has been minimized.

@arcanis arcanis added the external bug This issue highlights a bug in another project label Sep 17, 2020
@yarnbot

This comment has been minimized.

1 similar comment
@yarnbot
Copy link
Collaborator

yarnbot commented Sep 17, 2020

We couldn't reproduce your issue (all the assertions passed on master).

@clemyan
Copy link
Member Author

clemyan commented Sep 18, 2020

I have done some more investigation on this issue and have a workaround for now.

tl;dr: nyc 15 on Window injects its instrumentation by adding a script's dirname to NODE_PATH and then --require its basename in NODE_OPTIONS. Seems like Node cannot deal with a zipfs paths in NODE_PATH. Workaround: downgrade to node-preload@0.2.0 via resolutions.

So, nyc 15 is uses node-preload under the hood to inject its instrumentation. node-preload, in turn, injects itself by monkey-patching child_process to change the environment variables being passed to all spawned processes. In particular, it changes NODE_OPTION to add a --require flag.

However, Node before v12 cannot use a quoted path for --require, which means it cannot --require a path containing spaces. In these cases node-preload adds the path's dirname to NODE_PATH and basename to --require (since basename is always node-preload.js that would not cause problems).

In node-preload 0.2.0, that behavior is only enabled for Node pre-v12, Node v12+ uses a quoted path with --require. However that causes problems when a Node v12+ process spawns a Node pre-v12 process. node-preload 0.2.1 fixes that by always enabling that behavior for paths containing spaces, double quotes and/or backslashs.

But that means that behavior is always enabled on Windows since the paths to node-preload.js always have backslashs. That would add a zipfs path to NODE_PATH which Node seems to be unable to deal with.

As noted, I am working around that by downgrading to node-preload v0.2.0. Hopefully node-preload will always use the quote path method when Node v10 maintenance ends.

@clemyan clemyan closed this as completed Sep 18, 2020
ratson added a commit to admob-plus/admob-plus that referenced this issue Feb 2, 2021
ratson added a commit to admob-plus/admob-plus that referenced this issue Feb 2, 2021
JLHwung added a commit to babel/babel-loader that referenced this issue Aug 26, 2022
nicolo-ribaudo pushed a commit to babel/babel-loader that referenced this issue Aug 26, 2022
* migrate to Yarn 3

* fix broken cache test

* update CI node versions

* pin node-preload to 0.2.0

Due to yarnpkg/berry#1841
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working external bug This issue highlights a bug in another project unreproducible This issue cannot be reproduced on master
Projects
None yet
Development

No branches or pull requests

3 participants