[Case Study] Exports Support

[sparebytes]

• I'd be willing to implement a fix

Describe the bug

See:

• proposal-pkg-exports
• Node 13 Documentation

Node ^12.7.0 with the --experimental-modules flag and Node ^13.0.0 without the flag resolves modules differently than Yarn@2.0.0-rc19 with PNP

To Reproduce

See this project:
https://github.com/sparebytes/yarn2-pnp-exports-example

// packages/package-with-exports/package.json
{
    "name": "package-with-exports",
    "version": "1.0.0",
    "exports": {
        ".": "./main.js",
        "./sub/other": "./other.js"
    }
}

// Example 1
// Expected: packages/package-with-exports/main.js
// Actual: packages/package-with-exports/index.js
require("package-with-exports");

// Example 2
// Expected: packages/package-with-exports/other.js
// Actual: throws error
require("package-with-exports/sub/other");

Environment

• OS: Windows
• Node version ^12.7.0
• Yarn version 2.0.0-rc19

[sparebytes]

• Node 13.7 makes resolution more reliable and predictable.
  • Source Code for resolveExportsTarget
• There is a PR to expose the export conditions to loaders

[jkrems]

If nodejs/node#32869 lands, this likely should include support for NODE_ENV based resolution ("production" and "development").

[bgotink]

This one is hard to fit in the current PnP setup. Resolution is split into two parts:

• resolveToUnqualified
• resolveUnqualified

and neither is (at the moment) a good fit for package exports.

Why isn't `resolveToUnqualified` a good place for package export resolution?

Two big reasons:

• It requires reading from the package manifest, and resolveToUnqualified is not meant to touch the file system.
• The current behaviour of the function is to do a naive resolution. In other words: if I resolve pkg, I will get a result looking like /path/to/.yarn/cache/pkg-hash.zip/node_modules/pkg. If we add package export resolution here, that result will change to /path/to/.yarn/cache/pkg-hash.zip/node_modules/pkg/index.js assuming the pkg package has a main index.js export.

Why isn't `resolveUnqualified` a good place for package export resolution?

This is best shown using an example. Suppose we've got a package with manifest

{
  "name": "exported",
  "main": "./index.js",
  "exports": {
    "default": "./index.js",
    "./deep": "./lib/deep.js"
  }
}

The package contains three javascript files:

• index.js
• other.js
• lib/deep.js

If we implement package export resolution in resolveUnqualified we expect the following behaviour:

resolving: "exported"
resolveToUnqualified: /path/to/.yarn/cache/exported-hash.zip/node_modules/exported
resolveUnqualified: /path/to/.yarn/cache/exported-hash.zip/node_modules/exported/index.js

resolving: "exported/deep"
resolveToUnqualified: /path/to/.yarn/cache/exported-hash.zip/node_modules/exported/deep
resolveUnqualified: /path/to/.yarn/cache/exported-hash.zip/node_modules/exported/lib/deep.js

resolving: "./other" from the "exported" package's index.js file
resolveToUnqualified: /path/to/.yarn/cache/exported-hash.zip/node_modules/exported/other
resolveUnqualified: /path/to/.yarn/cache/exported-hash.zip/node_modules/exported/lib/other.js

resolving: "exported/other"
resolveToUnqualified: /path/to/.yarn/cache/exported-hash.zip/node_modules/exported/other
resolveUnqualified: this should throw because the "./other" export is not defined

• As the example above shows, the resolveUnqualified function must be able to distinguish between importing a file via a bare identifier or via a relative path. Currently that's impossible, because resolveUnqualified gets the unqualified path, which is identical in both cases.
• Notice how the path changed quite a bit when resolving exported/deep. Every path that is handed to resolveUnqualified will now result in us opening the package manifest to see whether the relative path passed in matches an export or not.

I currently see three options:

• Implement this behaviour in resolveUnqualified and give that function the information it needs (the imported identifier)
• Implement the behaviour in resolveToUnqualified and give that function the information it needs. In other words: add package exports (and main?) to the PnP data.
• Create a new function resolveExport that's called in between resolveToUnqualified and resolveUnqualified iff the package has defined exports. Whether a package has exports could be added to PnP data, or it could be read from the manifest. We could even store exports in the pnp data. If we decide not to store it in PnP data, we could cache the data so as not to load it anew for every resolution.

I'm inclined not to use the first option, it doesn't quite fit the description of that function.
The easiest solution would be the third option, reading data from the manifests (and caching?).

[yarnbot]

Hi! 👋

This issue looks stale, and doesn't feature the reproducible label - which implies that you didn't provide a working reproduction using Sherlock. As a result, it'll be closed in a few days unless a maintainer explicitly vouches for it or you edit your first post to include a formal reproduction (you can use the playground for that).

Note that we require Sherlock reproductions for long-lived issues (rather than standalone git repositories or similar) because we're a small team. Sherlock gives us the ability to check which bugs are still affecting the master branch at any given point, and decreases the amount of code we need to run on our own machines (thus leading to faster bug resolution faster). It helps us help you! 😃

If you absolutely cannot reproduce a bug on Sherlock (for example because it's a Windows-only issue), a maintainer will have to manually add the upholded label.