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

Subpath pattern exports do not work with arrays #49945

Closed
alshdavid opened this issue Sep 29, 2023 · 1 comment
Closed

Subpath pattern exports do not work with arrays #49945

alshdavid opened this issue Sep 29, 2023 · 1 comment

Comments

@alshdavid
Copy link

alshdavid commented Sep 29, 2023

Version

18.17.1

Platform

Darwin RG96396VM2 22.6.0 Darwin Kernel Version 22.6.0: Fri Sep 15 13:41:28 PDT 2023; root:xnu-8796.141.3.700.8~1/RELEASE_ARM64_T6000 arm64

What steps will reproduce the bug?

Repo with reproduction: https://github.com/alshdavid/node-exports-subpath-pattern-bug-example/tree/main

node ./src/cjs/index.mjs

How often does it reproduce? Is there a required condition?

Every time

What is the expected behavior? Why is that the expected behavior?

My package has a package.json describing subpath exports where the value is an array of patterns:

{
  "name": "pkg",
  "exports": {
    "./": {
      "import": "./import/index.js",
      "require": "./require/index.js"
    },
    "./*": {
      "import": "./import/*",
      "require": ["./require/*", "./require/*.js", "./require/*/index.js"]
    }
  }
}

The intention is to faithfully redirect imports as though they were being imported without any redirections such that the consumer would have a consumption experience indistinguishable from an unredirected import.

So from a CJS context:

const pkg = require('pkg')             /** resolves to */ 'node_modules/pkg/require/index.js'

const pkg = require('pkg/index')       /** resolves to */ 'node_modules/pkg/require/index'   
                                                /** or */ 'node_modules/pkg/require/index.js'
                                                /** or */ 'node_modules/pkg/require/index/index.js'

const pkg = require('pkg/index.js')    /** resolves to */ 'node_modules/pkg/require/index.js'

const pkg = require('pkg/foo.js')      /** resolves to */ 'node_modules/pkg/require/foo.js'

const pkg = require('pkg/foo')         /** resolves to */ 'node_modules/pkg/require/foo'    
                                                /** or */ 'node_modules/pkg/require/foo.js'
                                                /** or */ 'node_modules/pkg/require/foo/index.js'

const pkg = require('pkg/foo/bar')     /** resolves to */ 'node_modules/pkg/require/foo/bar' 
                                                /** or */ 'node_modules/pkg/require/foo/bar.js'
                                                /** or */ 'node_modules/pkg/require/foo/bar/index.js'

const pkg = require('pkg/foo/bar.js')  /** resolves to */ 'node_modules/pkg/require/foo/bar.js'

And from a MJS context:

import pkg from 'pkg'               /** resolves to */ 'node_modules/pkg/import/index.js'
import pkg from 'pkg/index'         /** not valid, requires extension */
import pkg from 'pkg/index.js'      /** resolves to */ 'node_modules/pkg/import/index.js'
import pkg from 'pkg/foo.js'        /** resolves to */ 'node_modules/pkg/import/foo.js'
import pkg from 'pkg/foo'           /** not valid, requires extension */
import pkg from 'pkg/foo/bar.js'    /** resolves to */ 'node_modules/pkg/import/foo/bar.js'

Due to export subpaths accepting an string | Array<string> - my expectation is that, when supplied with an Array<string>, Node would attempt to resolve each item in the array, returning the first successful result - only throwing when all paths have been tested and failed.

// Given
//  "./*": { "require": ["./require/*.js", "./require/*/index.js"] }

const pkg = require('pkg/foo')        

// Node should first attempt to resolve  'node_modules/pkg/require/foo.js', fail to find a file 
// then Node should attempt to resolve 'node_modules/pkg/require/foo/index.js'
// where it will throw Not Found if both patterns were unsuccessful

What Node appears to do instead is to test only the first item in the Array, throwing if that item does not exist.

This means that:

{ "exports": "./*": { "require": ["./require/*.js", "./require/*/index.js"] } }

Functions identically to

{ "exports": "./*": { "require": "./require/*.js" } }

I dug into the resolution and loading algorithm described here: https://nodejs.org/api/esm.html#resolution-and-loading-algorithm

Below is an excerpt of the relevant circuit

01 PACKAGE_EXPORTS_RESOLVE(packageURL, subpath, exports, conditions)
02   Otherwise, if exports is an Object and all keys of exports start with ".", then
03     Let matchKey be the string "./" concatenated with subpath.
04     Let resolved be the result of PACKAGE_IMPORTS_EXPORTS_RESOLVE( matchKey, exports, packageURL, false, conditions).
05     If resolved is not null or undefined, return resolved.
06       Throw a Package Path Not Exported error.
07 
08 PACKAGE_IMPORTS_EXPORTS_RESOLVE(matchKey, matchObj, packageURL, isImports, conditions)
09   Let expansionKeys be the list of keys of matchObj containing only a single "*", sorted by the sorting function PATTERN_KEY_COMPARE which orders in descending order of specificity.
10   For each key expansionKey in expansionKeys, do
11     Let patternBase be the substring of expansionKey up to but excluding the first "*" character.
12     If matchKey starts with but is not equal to patternBase, then
13       Let patternTrailer be the substring of expansionKey from the index after the first "*" character.
14       If patternTrailer has zero length, or if matchKey ends with patternTrailer and the length of matchKey is greater than or equal to the length of expansionKey, then
15         Let target be the value of matchObj[expansionKey].
16         Let patternMatch be the substring of matchKey starting at the index of the length of patternBase up to the length of matchKey minus the length of patternTrailer.
17         Return the result of PACKAGE_TARGET_RESOLVE(packageURL, target, patternMatch, isImports, conditions).
18 
19 PACKAGE_TARGET_RESOLVE(packageURL, target, patternMatch, isImports, conditions)
20    If target is a String, then
21      Let resolvedTarget be the URL resolution of the concatenation of packageURL and target.
22      Assert: resolvedTarget is contained in packageURL.
23      Return the URL resolution of resolvedTarget with every instance of "*" replaced with patternMatch.
24   Otherwise, if target is an Array, then
25 	   If _target.length is zero, return null.
26 	   For each item targetValue in target, do
27       Let resolved be the result of PACKAGE_TARGET_RESOLVE( packageURL, targetValue, patternMatch, isImports, conditions), continuing the loop on any Invalid Package Target error.
28       If resolved is undefined, continue the loop.
29       Return resolved.
31     Return or throw the last fallback resolution null return or error.
32   Otherwise, if target is null, return null.
33 Otherwise throw an Invalid Package Target error.

Line 26 describes looping through each item of the subpattern array then runs PACKAGE_TARGET_RESOLVE on that pattern as a string.

When testing each item, Line 23 will generate an import path by replacing the * with the package import path, returning the constructed string.

This jumps back to 29, returning the value to it caller - propagating upwards until the URL is tested against the filesystem and fails with a MODULE_NOT_FOUND error.

Node behaves exactly as the algorithm describes - however this means there is never a scenario where an Array is a usable subpath pattern because regardless of how many items are in the array, Node will always return the first item and ignore the subsequent patterns in the list.

If PACKAGE_RESOLVE added a condition for accepting an Array<string>, testing each pattern until success where PACKAGE_EXPORTS_RESOLVE could return a Array<string> - this would only affect imports like the ones I describe here.

This would not perform any worse than CJS imports that exclude extensions or import folders do currently - because Node already needs to test the FS for those alternative paths.

@alshdavid
Copy link
Author

Looks like this discussion already covers the topic: #37928

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

1 participant