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] PnP error with virtual packages #1524

Closed
1 task
bgotink opened this issue Jun 27, 2020 · 2 comments
Closed
1 task

[Bug] PnP error with virtual packages #1524

bgotink opened this issue Jun 27, 2020 · 2 comments
Labels
bug Something isn't working

Comments

@bgotink
Copy link
Member

bgotink commented Jun 27, 2020

I'm sorry for the vague issue title, but I don't really get what's going wrong so…

  • I'd be willing to implement a fix

Describe the bug

I've got a repo which contains, among other things, an eslint plugin that contains configuration that uses @typescript-eslint. The repo uses the plugin in its own configuration.

Trying to run eslint results in an error

Oops! Something went wrong! :(

ESLint: 7.3.1

Error: Failed to load parser '@typescript-eslint/parser' declared in '.eslintrc.yml » plugin:@my-pkg/base': Your application tried to access @typescript-eslint/typescript-estree, but it isn't declared in your dependencies; this makes the require call ambiguous and unsound.

Required package: @typescript-eslint/typescript-estree (via "@typescript-eslint/typescript-estree")
Required by: /private/var/folders/_d/ch2kc4h960d10cy_2c41qqzw0000gn/T/tmp.YaMQOVHcuF/.yarn/$$virtual/@my-pkg-eslint-plugin-virtual-a692f22303/0/cache/@typescript-eslint-parser-npm-3.4.0-5971bf1992-c2f13ed349.zip/node_modules/@typescript-eslint/parser/dist/

Require stack:
- /private/var/folders/_d/ch2kc4h960d10cy_2c41qqzw0000gn/T/tmp.YaMQOVHcuF/.yarn/$$virtual/@my-pkg-eslint-plugin-virtual-a692f22303/0/cache/@typescript-eslint-parser-npm-3.4.0-5971bf1992-c2f13ed349.zip/node_modules/@typescript-eslint/parser/dist/parser.js
- /private/var/folders/_d/ch2kc4h960d10cy_2c41qqzw0000gn/T/tmp.YaMQOVHcuF/.yarn/$$virtual/@my-pkg-eslint-plugin-virtual-a692f22303/0/cache/@typescript-eslint-parser-npm-3.4.0-5971bf1992-c2f13ed349.zip/node_modules/@typescript-eslint/parser/dist/index.js
- ...

Everything worked fine when both the repo root and pkg only depend on eslint and @typescript-eslint, but adding dev dependencies on @angular/core (which peer depends on rxjs and zone.js) suddenly leads to the error posted above.

I don't see what's wrong on my end or in the @typescript-eslint packages: dependencies appear to be listed correctly.
If it does turn out to be a mistake on my end / in one of the dependencies, the error message is quite confusing: the code in question is definitely not part of my root workspace.

To Reproduce

Clone https://github.com/bgotink/yarn-pnp-error-repro.git, run yarn eslint pkg.

@bgotink bgotink added the bug Something isn't working label Jun 27, 2020
@bgotink
Copy link
Member Author

bgotink commented Jul 6, 2020

I've been looking into this a bit.

TL;DR: I think we need to pass the found pnp API path through VirtualFS.resolveVirtual

It appears to me that the issue lies in the part of the pnpapi that finds the pnpapi for the issuer.
The first path in the require stack is a virtual path to a package in the workspace, so it's something akin to

/path/to/workspace/.yarn/$$virtual/scope-eslint-plugin-virtual-hash/1/packages/eslint-plugin/index.js

The pnpapi starts looking for a pnpapi to load and it finds one at

/path/to/workspace/.yarn/$$virtual/scope-eslint-plugin-virtual-hash/1/.pnp.js

and that's when stuff starts getting weird. That eslint plugin requires @typescript-eslint/parser, which is resolved to

/path/to/workspace/.yarn/$$virtual/scope-eslint-plugin-virtual-hash/0/cache/@typescript-eslint-parser-npm-3.4.0-5971bf1992-c2f13ed349.zip/node_modules/@typescript-eslint/parser/dist/index.js

When that file requires @typescript-eslint/typescript-estree things go wrong:

  • The pnpapi found for that issuer is /path/to/workspace/.pnp.js
  • .yarn/$$virtual/scope-eslint-plugin-virtual-hash/0/ is not a registered package location in the .pnp.js file, .yarn/$$virtual/scope-eslint-plugin-virtual-hash/1/packages/eslint-plugin/ is.

-> issuer is the top-level workspace, which doesn't depend on @typescript-eslint/typescript-estree.

I've hacked together a dirty dirty dirty solution:

hacky hacky diff
diff --git a/.pnp.js b/.pnp.js
index eb20628..c1d3e60 100755
--- a/.pnp.js
+++ b/.pnp.js
@@ -20129,30 +20129,36 @@ function makeManager(pnpapi, opts) {
 
   function findApiPathFor(modulePath) {
     const start = sources_path/* ppath.resolve */.y1.resolve(sources_path/* npath.toPortablePath */.cS.toPortablePath(modulePath));
     let curr;
     let next = start;
 
     do {
       curr = next;
       const cached = findApiPathCache.get(curr);
 
       if (cached !== undefined) {
         addToCache(start, curr, cached);
         return cached;
       }
 
+      const match = curr.match(/\/\$\$virtual\/[^/]+\/1\/?$/);
+      if (match != null) {
+        next = curr.slice(0, match.index);
+        continue;
+      }
+
       const candidate = sources_path/* ppath.join */.y1.join(curr, `.pnp.js`);
 
       if (xfs.existsSync(candidate) && xfs.statSync(candidate).isFile()) {
         addToCache(start, curr, candidate);
         return candidate;
       }
 
       const cjsCandidate = sources_path/* ppath.join */.y1.join(curr, `.pnp.cjs`);
 
       if (xfs.existsSync(cjsCandidate) && xfs.statSync(cjsCandidate).isFile()) {
         addToCache(start, curr, cjsCandidate);
         return cjsCandidate;
       }
 
       next = sources_path/* ppath.dirname */.y1.dirname(curr);

which is basically a simplified version of VirtualFS.resolveVirtual and now eslint seems to work without issue.
The packages/eslint-plugin package has a peerDependency on typescript with a devDependency on 3.4.5, while the top-level has a devDependency on the packages/eslint-plugin package and typescript 4.0.0-beta. Eslint correctly loads with the 4.0.0-beta version.

I've got a small test script that mocks the behaviour of eslint:

test script
const {createRequire} = require('module');

const packages = [
  '@scope/eslint-plugin',
  '@typescript-eslint/parser',
  '@typescript-eslint/typescript-estree',
];

let loc = __filename;

for (const pkg of packages) {
  const require = createRequire(loc);

  loc = require.resolve(pkg);
  console.log(`${pkg} => ${loc}`);
  console.log('');
}

Without the hack above, this yields

@scope/eslint-plugin => /Users/bram/Workspaces/projectName/.yarn/$$virtual/@scope-eslint-plugin-virtual-b9883f9a9a/1/packages/eslint-plugin/index.js

@typescript-eslint/parser => /Users/bram/Workspaces/projectName/.yarn/$$virtual/@scope-eslint-plugin-virtual-b9883f9a9a/0/cache/@typescript-eslint-parser-npm-3.4.0-5971bf1992-c2f13ed349.zip/node_modules/@typescript-eslint/parser/dist/index.js

/Users/bram/Workspaces/projectName/.pnp.js:19230
    throw firstError;
    ^

Error: Your application tried to access @typescript-eslint/typescript-estree, but it isn't declared in your dependencies; this makes the require call ambiguous and unsound.

Required package: @typescript-eslint/typescript-estree (via "@typescript-eslint/typescript-estree")
Required by: /Users/bram/Workspaces/projectName/.yarn/$$virtual/@scope-eslint-plugin-virtual-b9883f9a9a/0/cache/@typescript-eslint-parser-npm-3.4.0-5971bf1992-c2f13ed349.zip/node_modules/@typescript-eslint/parser/dist/

Require stack:
- /Users/bram/Workspaces/projectName/.yarn/$$virtual/@scope-eslint-plugin-virtual-b9883f9a9a/0/cache/@typescript-eslint-parser-npm-3.4.0-5971bf1992-c2f13ed349.zip/node_modules/@typescript-eslint/parser/dist/index.js
    at internalTools_makeError (/Users/bram/Workspaces/projectName/.pnp.js:18975:34)
    at resolveToUnqualified (/Users/bram/Workspaces/projectName/.pnp.js:19895:21)
    at resolveRequest (/Users/bram/Workspaces/projectName/.pnp.js:19987:29)
    at Object.resolveRequest (/Users/bram/Workspaces/projectName/.pnp.js:20053:26)
    at Function.external_module_.Module._resolveFilename (/Users/bram/Workspaces/projectName/.pnp.js:19207:34)
    at Function.resolve (internal/modules/cjs/helpers.js:79:19)
    at Object.<anonymous> (/Users/bram/Workspaces/projectName/test.js:14:17)
    at Module._compile (internal/modules/cjs/loader.js:1200:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1220:10)
    at Module.load (internal/modules/cjs/loader.js:1049:32)

with the hack it's

@scope/eslint-plugin => /Users/bram/Workspaces/projectName/.yarn/$$virtual/@scope-eslint-plugin-virtual-b9883f9a9a/1/packages/eslint-plugin/index.js

@typescript-eslint/parser => /Users/bram/Workspaces/projectName/.yarn/$$virtual/@typescript-eslint-parser-virtual-cb0ef37567/0/cache/@typescript-eslint-parser-npm-3.4.0-5971bf1992-c2f13ed349.zip/node_modules/@typescript-eslint/parser/dist/index.js

@typescript-eslint/typescript-estree => /Users/bram/Workspaces/projectName/.yarn/$$virtual/@typescript-eslint-typescript-estree-virtual-4b8b60010f/0/cache/@typescript-eslint-typescript-estree-npm-3.4.0-598abcf734-96cb85cd30.zip/node_modules/@typescript-eslint/typescript-estree/dist/index.js

So, long story short: I think we need to pass the found pnp API path through VirtualFS.resolveVirtual

@arcanis
Copy link
Member

arcanis commented Jul 7, 2020

Oh wow, that's a fun one 😅 Thanks for digging, I'll make a fix 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants