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

Fix subpath imports with Yarn PnP #2547

Merged
merged 9 commits into from Sep 29, 2022
Merged

Fix subpath imports with Yarn PnP #2547

merged 9 commits into from Sep 29, 2022

Conversation

evanw
Copy link
Owner

@evanw evanw commented Sep 14, 2022

Node has a little-used feature called subpath imports which are package-internal imports that start with # and that go through the imports map in package.json. Previously esbuild had a bug that caused esbuild to not handle these correctly in packages installed via Yarn's "Plug'n'Play" installation strategy. The problem was that subpath imports were being checked after Yarn PnP instead of before. This PR reorders these checks, which should allow subpath imports to work in this case.

Note: Tests are currently failing due to a bug in Yarn itself: yarnpkg/berry#3843. This PR is blocked on that bug being fixed.

Fixes #2545

@F43nd1r
Copy link

F43nd1r commented Sep 28, 2022

Heads up: yarnpkg/berry#3843 was fixed in yarnpkg/berry#4862

@evanw
Copy link
Owner Author

evanw commented Sep 29, 2022

I tried updating to Yarn 4.0.0-rc.22 but it looks like it has a bug regarding paths on Windows. The package manifest that Yarn generates contains ../C:/ which I believe is invalid on Windows:

["@tokenizer/token", [\
  ["npm:0.3.0", {\
    "packageLocation": "../../../../../../C:/Users/runneradmin/AppData/Local/Yarn/Berry/cache/@tokenizer-token-npm-0.3.0-4441352cc5-9.zip/node_modules/@tokenizer/token/",\
    "packageDependencies": [\
      ["@tokenizer/token", "npm:0.3.0"]\
    ],\
    "linkType": "HARD"\
  }]\
]],\

This happens because GitHub Actions puts the repo on the D:\ drive but Yarn puts its caches on the C:\ drive, and Yarn is attempting to use a relative path instead of an absolute path to refer to a file on another drive. You aren't allowed to navigate between drives using a relative path on Windows. Yarn would have to either use an absolute path instead of a relative path here, or Yarn would have to put their cache on the same drive as the repo.

It looks like I need to wait for this issue to be fixed in a future release of Yarn before landing this change.

Edit: This is the same part of the package manifest from Yarn 3.2.3 for reference. This one works fine because it's located on the same drive:

["@tokenizer/token", [\
  ["npm:0.3.0", {\
    "packageLocation": "./.yarn/cache/@tokenizer-token-npm-0.3.0-4441352cc5-1d575d02d2.zip/node_modules/@tokenizer/token/",\
    "packageDependencies": [\
      ["@tokenizer/token", "npm:0.3.0"]\
    ],\
    "linkType": "HARD"\
  }]\
]],\

@evanw
Copy link
Owner Author

evanw commented Sep 29, 2022

It looks like maybe Yarn 4 changed the default of enableGlobalCache: false to enableGlobalCache: true which doesn't work on Windows if there are multiple drives. But perhaps I can explicitly set enableGlobalCache: false for tests to work around this bug in Yarn 4.

@evanw evanw merged commit c55000d into master Sep 29, 2022
@evanw evanw deleted the yarnpnp-subpath-imports branch September 29, 2022 14:34
@merceyz
Copy link

merceyz commented Sep 29, 2022

It works in Yarn because internally it treats it as a posix path where there are no such limitations, it only turns it into a win32 path when passing it to the fs module.

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

Successfully merging this pull request may close these issues.

Could not resolve "#node-web-compat" - subpath imports with esbuild and yarn pnp
3 participants