metro icon indicating copy to clipboard operation
metro copied to clipboard

Package Exports resolution differs from Node in 2 edge cases

Open jbroma opened this issue 11 months ago β€’ 3 comments

Do you want to request a feature or report a bug? Bug

Hey metro team! πŸ‘‹

I was working on aligning Re.Pack's resolution setup to follow metro-resolver with RN defaults closely and found these 2 edge-cases were metro-resolver behaves in a weird way when dealing with Package Exports:

1. Root Array Shorthand

For the Root Array shorthand, the implementation differs when compared to Node.js or Webpack's enhanced-resolve specification:

{
  "name": "@metro-package-exports/root-shorthand",
  "exports": ["./index.js"]
}

Results:

For the import specifier: @metro-package-exports/root-shorthand

  • Node.js: Resolves to index.js inside of @metro-package-exports/root-shorthand.
  • enhanced-resolve: Identically resolves to index.js within @metro-package-exports/root-shorthand.
  • metro-resolver: Fails to resolve, falling back to file resolution and ultimately resolving to the @metro-package-exports/root-shorthand directory itself.

2. Subpath Patterns in exports

When using subpath patterns in exports to match the import specifier, the behaviors observed are as follows:

{
  "name": "@metro-package-exports/restricted",
  "exports": {
    ".": "./index.js",
    "./features/*.js": "./features/*.js",
    "./features/internal/*.js": null
  }
}

Results:

For the import specifier: @metro-package-exports/restricted/features/internal/b.js

  • Node.js: Fails to resolve as b.js is not exported from @metro-package-exports/restricted.
  • enhanced-resolve: Also fails to resolve for the same reason as Node.js.
  • metro-resolver: Successfully resolves to @metro-package-exports/restricted/features/internal/b.js without falling back to file resolution.

Reproduction repository

https://github.com/jbroma/metro-package-exports

Setup

  1. Install dependencies via yarn v1 (classic).
  2. Make the test scripts within the tester directory executable by running chmod +x on them.
  3. Execute test-root-shorthand.sh & test-restricted.sh scripts.

What is the expected behavior?

Resolution in edge cases above should follow Node.js resolution algorithm

jbroma avatar Mar 19 '24 21:03 jbroma

Few other insights about Package Exports implementation & tests:

  1. When multiple entires in root array shorthand are defined the resolvers behave in a following way:

    • node: looks for first valid specifier (beginning with ./), and returns it, even when it's non-existent
    • enhanced-resolve: similar to node, but looks for first valid entry that actually exists
    • metro-resolver: can't tell because case from the issue fails
  2. IMO, these test cases should be marked as [nonstrict] as they rely on fallback mechanism:

  • should resolve "exports" target directly > without expanding sourceExts
  • should resolve "exports" target directly > without expanding platform-specific extensions

FYI, im willing to submit a PR for this after we agree on the spec.

jbroma avatar Mar 20 '24 12:03 jbroma

Thanks @jbroma!

1. Root Array Shorthand

Wow, essentially these implementations have a special-case expansion of ["./index.js"] to {".": "./index.js"} β€”Β happy to align βœ….

2. Subpath Patterns in exports

Yup, also looks incorrect βœ…. I don't think we have a test against that behaviour at all.


IMO, these test cases should be marked as [nonstrict] as they rely on fallback mechanism:

  • should resolve "exports" target directly > without expanding sourceExts
  • should resolve "exports" target directly > without expanding platform-specific extensions

These tests are for in-spec behaviour, so I think this is right as-is. However, the assertions seem slightly wrong (unless I'm misremembering) β€”Β they should be index-exports.js. This test needs a little fixing.


Yes please to PRs! πŸ™ŒπŸ» Recommend splitting into one PR per fix, and we'll happily look through :).

huntie avatar Mar 20 '24 17:03 huntie

@huntie all tasks from the issue handled in the PRs above.

jbroma avatar Mar 21 '24 22:03 jbroma