Virtual paths

[arcanis]

A couple things I noticed:

● [DEBUG] Resolving import "@yarnpkg/cli" in directory "/Users/mael.nison/berry/.yarn/__virtual__/@yarnpkg-plugin-stage-virtual-7a1205884d/1/packages/plugin-stage/sources/commands" of type "import-statem$

  Read 1 entry for directory "/Users/mael.nison/berry/.yarn/__virtual__/@yarnpkg-plugin-stage-virtual-7a1205884d/1/packages/plugin-stage/sources/commands"
  Using Yarn PnP manifest from "/Users/mael.nison/berry/.yarn/__virtual__/@yarnpkg-plugin-stage-virtual-7a1205884d/1/.pnp.cjs" to resolve "@yarnpkg/cli"
    Found parent locator: ["@yarnpkg/plugin-stage", "workspace:packages/plugin-stage"]
    Found parent package at "./packages/plugin-stage/"
    Found dependency locator: ["@yarnpkg/cli", "virtual:86c95fabbcd56c56f5f2d2e080e64a1095e3fe233877aa9f7958f317f88a95627e0be2765e89c0cff02c9f08f27b64b7cbc9d5c3960c1df509d5e6ea98cca4f4#workspace:packages$
    Found package "@yarnpkg/cli" at "./.yarn/__virtual__/@yarnpkg-cli-virtual-ce4dc31355/1/packages/yarnpkg-cli/"
    Resolved "@yarnpkg/cli" via Yarn PnP to "/Users/mael.nison/berry/.yarn/__virtual__/@yarnpkg-plugin-stage-virtual-7a1205884d/1/.yarn/__virtual__/@yarnpkg-cli-virtual-ce4dc31355/1/packages/yarnpkg-cli/$
  The import "/Users/mael.nison/berry/.yarn/__virtual__/@yarnpkg-plugin-stage-virtual-7a1205884d/1/.yarn/__virtual__/@yarnpkg-cli-virtual-ce4dc31355/1/packages/yarnpkg-cli/" is being treated as an absolu$
  Attempting to load "/Users/mael.nison/berry/.yarn/__virtual__/@yarnpkg-plugin-stage-virtual-7a1205884d/1/.yarn/__virtual__/@yarnpkg-cli-virtual-ce4dc31355/1/packages/yarnpkg-cli/" as a file
    Failed to read directory "/Users/mael.nison/berry/.yarn/__virtual__/@yarnpkg-plugin-stage-virtual-7a1205884d/1/.yarn/__virtual__/@yarnpkg-cli-virtual-ce4dc31355/1/packages/yarnpkg-cli": open /Users/m$
  Attempting to load "/Users/mael.nison/berry/.yarn/__virtual__/@yarnpkg-plugin-stage-virtual-7a1205884d/1/.yarn/__virtual__/@yarnpkg-cli-virtual-ce4dc31355/1/packages/yarnpkg-cli/" as a directory
    Read 34 entries for directory "/Users/mael.nison/berry/.yarn/__virtual__/@yarnpkg-plugin-stage-virtual-7a1205884d/1"

• It seems that the virtual paths aren't present before checking which package owns a path (FIND_LOCATOR). As a result, Esbuild detects the wrong parent locator: ["@yarnpkg/plugin-stage", "workspace:packages/plugin-stage"] (it should instead be something like ["@yarnpkg/plugin-stage", "virtual:<some hash>"])

  • Do you think it's worth adding to the JSON tests, even if virtuals are theoretically an implementation detail?

• I'm not sure the virtual part of virtual paths is always stripped; cf the following log line, where you can see .yarn appearing twice: "/Users/mael.nison/berry/.yarn/__virtual__/@yarnpkg-plugin-stage-virtual-7a1205884d/1/.yarn/__virtual__/@yarnpkg-cli-virtual-ce4dc31355/1/packages/yarnpkg-cli/

  • This may be caused by the spec being missing a detail (I'm adding a clarification to Fixes errors in the PnP spec yarnpkg/berry#4724): all the packageLocation fields are always relative to the manifest file, not the importer.

To reproduce both of these issues from the Yarn repository:

yarn up esbuild@0.15.0
sed -i.bak -e "s/, pnpPlugin()//" packages/yarnpkg-builder/sources/commands/build/bundle.ts
sed -i.bak -e "s/silent/debug/" packages/yarnpkg-builder/sources/commands/build/bundle.ts
yarn build:cli

[merceyz]

Looking at the implementation in e870ec5 I see it checks for __virtual__ but in Yarn v2, which it looks like ESBuild wants to support, the paths used $$virtual, we changed them in yarnpkg/berry#2595.

Ref yarnpkg/berry#4724 (comment)

[evanw]

Thanks for the feedback!

Do you think it's worth adding to the JSON tests, even if virtuals are theoretically an implementation detail?

Yes, that would be very helpful for implementing this. Especially because it sounds like virtual paths interact with manifest path resolution in a specific way.

• This may be caused by the spec being missing a detail (I'm adding a clarification to Fixes errors in the PnP spec yarnpkg/berry#4724): all the packageLocation fields are always relative to the manifest file, not the importer.

Looking at the implementation in e870ec5 I see it checks for __virtual__ but in Yarn v2, which it looks like ESBuild wants to support, the paths used $$virtual, we changed them in yarnpkg/berry#2595.

Sounds good. I will wait to implement these points until yarnpkg/berry#4724 lands and the spec is updated.

[evanw]

• I'm not sure the virtual part of virtual paths is always stripped; cf the following log line, where you can see .yarn appearing twice: "/Users/mael.nison/berry/.yarn/__virtual__/@yarnpkg-plugin-stage-virtual-7a1205884d/1/.yarn/__virtual__/@yarnpkg-cli-virtual-ce4dc31355/1/packages/yarnpkg-cli/

This happens because __virtual__ in esbuild is handled as an implementation detail of the file system layer. From what I understood, virtual paths are supposed to appear like real paths to everything that uses the file system because the intent is for __virtual__ paths to be treated like duplicate files (even though they are actually the same file).

However, that causes a problem for the FIND_CLOSEST_PNP_MANIFEST algorithm:

FIND_CLOSEST_PNP_MANIFEST(url)

1. Let manifest be null
2. Let directoryPath be the directory for url
3. Let pnpPath be directoryPath concatenated with /.pnp.cjs
4. If pnpPath exists on the filesystem, then
   1. Let pnpDataPath be directoryPath concatenated with /.pnp.data.json
   2. Set manifest to JSON.parse(readFile(pnpDataPath))
   3. Set manifest.dirPath to directoryPath
   4. Return manifest
5. Otherwise, if directoryPath is /, then
   1. Return null
6. Otherwise,
   1. Return FIND_PNP_MANIFEST(directoryPath)

For example, imagine a project with a manifest at /project/.pnp.cjs and a source file /project/.yarn/__virtual__/pkg/1/foo.js that imports bar. This algorithm says the closest PnP manifest is /project/.yarn/__virtual__/pkg/1/.pnp.cjs, not /project/.pnp.cjs, which results in sort of a fractal file system. Doing PnP path resolution on this leads to paths like /project/.yarn/__virtual__/pkg/1/.yarn/__virtual__/pkg/1/foo which then fails because esbuild doesn't currently handle multiple __virtual__ path components.

I was able to get it to work correctly by modifying esbuild's equivalent behavior for FIND_CLOSEST_PNP_MANIFEST to ignore manifests in virtual directories. This makes sense to me because a manifest in a virtual directory is just a copy of one from a parent directory, so by ignoring the inner one we will automatically pick up on the outer one. You could consider adding a note about that in the specification of FIND_CLOSEST_PNP_MANIFEST.

[merceyz]

Looks like the spec doesn't quite match our implementation, our FIND_PNP_MANIFEST loops over all the PnP manifests we know about already and calls their FIND_LOCATOR, if they all return null then we check the filesystem.
https://github.com/yarnpkg/berry/blob/15c481dce19f1a7e56b875b96f11c9ef67dc4c2d/packages/yarnpkg-pnp/sources/loader/makeManager.ts#L98-L164