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

pnp: esbuild@0.15.3 unable to resolve esm packages #2473

Closed
YXL76 opened this issue Aug 16, 2022 · 8 comments
Closed

pnp: esbuild@0.15.3 unable to resolve esm packages #2473

YXL76 opened this issue Aug 16, 2022 · 8 comments

Comments

@YXL76
Copy link

YXL76 commented Aug 16, 2022

To Reproduce

yarn init -2
yarn add strtok3 esbuild
echo "import * as strtok3 from 'strtok3';" > index.js
yarn esbuild index.js --bundle

Result

✘ [ERROR] Could not resolve "strtok3"

    index.js:1:25:
      1 │ import * as strtok3 from 'strtok3';
        ╵                          ~~~~~~~~~

  You can mark the path "strtok3" as external to exclude it from the bundle, which will remove this
  error.
@evanw
Copy link
Owner

evanw commented Aug 16, 2022

Thanks for the report. I can reproduce the issue. It looks like Yarn PnP disables interpretation of exports in esbuild. That's problematic for this package because it doesn't have a fallback main field.

The reason this is happening is because Yarn PnP's specification says to run node's module resolution algorithm afterward on the result (specifically the last line):

NM_RESOLVE(specifier, parentURL)

  1. This function is specified in the Node.js documentation

PNP_RESOLVE(specifier, parentURL)

  1. Let resolved be undefined
  2. If specifier is a Node.js builtin, then
    1. Set resolved to specifier itself and return it
  3. Otherwise, if specifier is either an absolute path or a path prefixed with "./" or "../", then
    1. Set resolved to NM_RESOLVE(specifier, parentURL) and return it
  4. Otherwise,
    1. Note: specifier is now a bare identifier
    2. Let unqualified be RESOLVE_TO_UNQUALIFIED(specifier, parentURL)
    3. Set resolved to NM_RESOLVE(unqualified, parentURL)

However, node's module resolution algorithm only interprets exports for package paths (specifically it stops at step 3):

ESM_RESOLVE(specifier, parentURL)

  1. Let resolved be undefined.
  2. If specifier is a valid URL, then
    1. Set resolved to the result of parsing and reserializing
      specifier as a URL.
  3. Otherwise, if specifier starts with "/", "./", or "../", then
    1. Set resolved to the URL resolution of specifier relative to
      parentURL.
  4. Otherwise, if specifier starts with "#", then
    1. Set resolved to the result of
      PACKAGE_IMPORTS_RESOLVE(specifier,
      parentURL, defaultConditions).
  5. Otherwise,
    1. Note: specifier is now a bare specifier.
    2. Set resolved the result of
      PACKAGE_RESOLVE(specifier, parentURL).
  6. Let format be undefined.
  7. If resolved is a "file:" URL, then
    1. If resolved contains any percent encodings of "/" or "\" ("%2F"
      and "%5C" respectively), then
      1. Throw an Invalid Module Specifier error.
    2. If the file at resolved is a directory, then
      1. Throw an Unsupported Directory Import error.
    3. If the file at resolved does not exist, then
      1. Throw a Module Not Found error.
    4. Set resolved to the real path of resolved, maintaining the
      same URL querystring and fragment components.
    5. Set format to the result of ESM_FILE_FORMAT(resolved).
  8. Otherwise,
    1. Set format the module format of the content type associated with the
      URL resolved.
  9. Load resolved as module format, format.

The paths coming out of Yarn PnP are no longer package paths because they have been resolved to a directory by Yarn PnP so esbuild doesn't interpret the exports map. Yarn's implementation appears to actually use a custom implementation of node's module resolution algorithm (called pnpapi.resolveRequest) instead of actually using the built-in one, and there are apparently some differences.

Pinging @arcanis regarding what to do about this.

@arcanis
Copy link

arcanis commented Aug 16, 2022

The way we handle this is that we make the "is it a package import" check on the specifier, not the resolution / unqualified path (if it's a package import, we then use the unqualified path to read the package.json and its exports field, like in Node). This is handled here.

@merceyz
Copy link

merceyz commented Aug 16, 2022

The fix for this issue broke importing workspaces, the verbose logs also show it doing a lot of file IO trying to locate stuff in node_modules which it will never find.

@evanw
Copy link
Owner

evanw commented Aug 16, 2022

Can you provide an example test case? I’m building up a suite of Yarn PnP tests for CI and that would be very helpful to have.

@merceyz
Copy link

merceyz commented Aug 16, 2022

I can indeed, opened #2476 with a reproduction to track the regression.

@YXL76
Copy link
Author

YXL76 commented Aug 17, 2022

strtok3 has subpath exports. esbuild@0.15.4 still cannot resolve it.

import * as strtok3 from 'strtok3/core';

@evanw
Copy link
Owner

evanw commented Aug 17, 2022

strtok3 has subpath exports. esbuild@0.15.4 still cannot resolve it.

Thanks for calling this out. I have realized this as well, and already have a fix. I'm currently working on getting it ready to be released.

@merceyz
Copy link

merceyz commented Aug 17, 2022

the verbose logs also show it doing a lot of file IO trying to locate stuff in node_modules which it will never find.
#2473 (comment)

Was able to track this down, opened #2481.

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

No branches or pull requests

4 participants