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

unstable_enablePackageExports does not resolve the root '.' exports alias #965

Closed
shamilovtim opened this issue Apr 6, 2023 · 5 comments · May be fixed by vladovello/MobileProject#56 or vladovello/MobileProject#57

Comments

@shamilovtim
Copy link

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

What is the current behavior?

  "exports": {
    ".": {
      "types": "./dist/src/index.d.ts",
      "import": "./dist/src/index.js"
    },
    "./chunker": {
      "types": "./dist/src/chunker/index.d.ts",
      "import": "./dist/src/chunker/index.js"
    },
    "./layout": {
      "types": "./dist/src/layout/index.d.ts",
      "import": "./dist/src/layout/index.js"
    }
  }

metro is 0.76.0
Given that unstable_enablePackageExports is true.
Given the above exports in a package.json . It seems that metro does not recognize that . is the main package. I receive the following error:

error: Error: While trying to resolve module `ipfs-unixfs-importer` from file `/Users/admin/Documents/Development/block/rc1wallet/node_modules/@tbd54566975/dwn-sdk-js/dist/esm/src/store/data-store-level.js`, the package `/Users/admin/Documents/Development/block/rc1wallet/node_modules/ipfs-unixfs-importer/package.json` was successfully found. However, this package itself specifies a `main` module field that could not be resolved (`/Users/admin/Documents/Development/block/rc1wallet/node_modules/ipfs-unixfs-importer/index`. Indeed, none of these files exist:

  * /Users/admin/Documents/Development/block/rc1wallet/node_modules/ipfs-unixfs-importer/index(.native|.ios.js|.native.js|.js|.ios.jsx|.native.jsx|.jsx|.ios.json|.native.json|.json|.ios.ts|.native.ts|.ts|.ios.tsx|.native.tsx|.tsx)
  * /Users/admin/Documents/Development/block/rc1wallet/node_modules/ipfs-unixfs-importer/index/index(.native|.ios.js|.native.js|.js|.ios.jsx|.native.jsx|.jsx|.ios.json|.native.json|.json|.ios.ts|.native.ts|.ts|.ios.tsx|.native.tsx|.tsx)
    at DependencyGraph.resolveDependency (/Users/admin/Documents/Development/block/rc1wallet/node_modules/metro/src/node-haste/DependencyGraph.js:283:17)
    at Object.resolve (/Users/admin/Documents/Development/block/rc1wallet/node_modules/metro/src/lib/transformHelpers.js:171:21)
    at Graph._resolveDependencies (/Users/admin/Documents/Development/block/rc1wallet/node_modules/metro/src/DeltaBundler/Graph.js:420:35)
    at Graph._processModule (/Users/admin/Documents/Development/block/rc1wallet/node_modules/metro/src/DeltaBundler/Graph.js:218:38)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async Graph._addDependency (/Users/admin/Documents/Development/block/rc1wallet/node_modules/metro/src/DeltaBundler/Graph.js:316:20)
    at async Promise.all (index 2)
    at async Graph._processModule (/Users/admin/Documents/Development/block/rc1wallet/node_modules/metro/src/DeltaBundler/Graph.js:263:5)
    at async Graph._addDependency (/Users/admin/Documents/Development/block/rc1wallet/node_modules/metro/src/DeltaBundler/Graph.js:316:20)
    at async Promise.all (index 5)

If the current behavior is a bug, please provide the steps to reproduce and a minimal repository on GitHub that we can yarn install and yarn test.
https://github.com/shamilovtim/metro76exportsrepro
bug is present in package @tbd54566975/dwn-sdk-js

What is the expected behavior?
Expect metro to resolve "." : { } inside of exports and recognize that this provide the main resolution, which would lead it to finding index.js in the correct spot.

Please provide your exact Metro configuration and mention your Metro, node, yarn/npm version and operating system.

Metro config

const config = {
  resolver: {
    unstable_enablePackageExports: true,
    unstable_conditionNames: ['react-native', 'require', 'import'],
  },
};

module.exports = mergeConfig(getDefaultConfig(__dirname), config);

metro 0.76.0
yarn 1.22.18
node v16.18.0
npm 9.4.2
macOS latest

@huntie
Copy link
Member

huntie commented Apr 7, 2023

Thanks for the bug report! Will investigate next week 👀

  • We have an existing test case for conditional exports on the main entry point today, and this is working inside Meta's codebase — so I think this might be failing for a more complex reason here.
  • On a quick attempt at reproing this (thanks for sharing) against Metro's main branch (which has some recent fixes clearing out the incorrect @babel/runtime warnings), I'm unable to replicate the issue — but will look deeper to make sure.

@shamilovtim
Copy link
Author

@huntie is resolverMainFields not compatible with unstable_enablePackageExports? It seems when I try to configure these two together, for example with:

const config = {
  resolver: {
    resolverMainFields: ['react-native', 'main'],
    unstable_enablePackageExports: true,
    unstable_conditionNames: ['react-native', 'require', 'import'],
  },
};

This breaks bundling for me and exports is no longer usable. Adding exports manually to resolverMainFields creates a different class of error in my application. I think this could be related to the source of my original bug.

@shamilovtim
Copy link
Author

Updating https://github.com/shamilovtim/metro76exportsrepro with the latest repro demonstrating my above comment. After all of the babel warnings you should receive the following error:

error: Error: While trying to resolve module `ipfs-unixfs-importer` from file `/Users/admin/Documents/Development/block/rc1wallet/node_modules/@tbd54566975/dwn-sdk-js/dist/esm/src/store/data-store-level.js`, the package `/Users/admin/Documents/Development/block/rc1wallet/node_modules/ipfs-unixfs-importer/package.json` was successfully found. However, this package itself specifies a `main` module field that could not be resolved (`/Users/admin/Documents/Development/block/rc1wallet/node_modules/ipfs-unixfs-importer/index`. Indeed, none of these files exist:

@huntie
Copy link
Member

huntie commented Apr 11, 2023

is resolverMainFields not compatible with unstable_enablePackageExports?

These are independent options. If unstable_enablePackageExports is enabled, and the package has an "exports" field, then only unstable_conditionNames is considered.

However if that fails, then we fall back to resolverMainFields in a second pass. (Note: We have incoming docs that will describe this behaviour! You can dig into the RFC in the meantime.)


Okay, I've reproduced the bug! It's related to how we locate a package.json for a given filePath (used by both "exports" and mainFields resolution). Skips over that package manifest entirely, at least for the "exports" resolution pass :|.

➡️ Fix incoming.

However, after fixing this, it's worth noting that other deps in your project seem to widely depend on Node.js APIs, which could independently be problematic.

  • classic-level — uses "main" only.
  • readable-stream — uses "browser" field — would need to be included in your resolverMainFields config.

image

@shamilovtim
Copy link
Author

shamilovtim commented Apr 11, 2023

is resolverMainFields not compatible with unstable_enablePackageExports?

These are independent options. If unstable_enablePackageExports is enabled, and the package has an "exports" field, then only unstable_conditionNames is considered.

However if that fails, then we fall back to resolverMainFields in a second pass. (Note: We have incoming docs that will describe this behaviour! You can dig into the RFC in the meantime.)

Okay, I've reproduced the bug! It's related to how we locate a package.json for a given filePath (used by both "exports" and mainFields resolution). Skips over that package manifest entirely, at least for the "exports" resolution pass :|. (Update: This was previously inconsequential for mainFields, since redirections only support subpaths. This bug is entirely valid as reported for "exports"! ✅)

➡️ Fix incoming.

Awesome, glad you found it!

However, after fixing this, it's worth noting that other deps in your project seem to widely depend on Node.js APIs, which could independently be problematic.

  • classic-level — uses "main" only.
  • readable-stream — uses "browser" field — would need to be included in your resolverMainFields config.
image

Appreciate you pointing that out. I am aware of it and it's a huge pain in the butt. I'm asking the upstream guys to separate out the node implementation so I can get one that doesn't include stuff like classic-level. Readable Stream should be polyfilled but the polyfill seems to depend on node's readable-stream, which is strange. I hope to get this sorted out in the next couple of days.

fuxingloh pushed a commit to levaintech/keychain that referenced this issue Jun 20, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [metro](https://togithub.com/facebook/metro) | [`0.76.0` ->
`0.76.6`](https://renovatebot.com/diffs/npm/metro/0.76.0/0.76.6) |
[![age](https://badges.renovateapi.com/packages/npm/metro/0.76.6/age-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://badges.renovateapi.com/packages/npm/metro/0.76.6/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://badges.renovateapi.com/packages/npm/metro/0.76.6/compatibility-slim/0.76.0)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://badges.renovateapi.com/packages/npm/metro/0.76.6/confidence-slim/0.76.0)](https://docs.renovatebot.com/merge-confidence/)
|
| [metro-resolver](https://togithub.com/facebook/metro) | [`0.76.0` ->
`0.76.6`](https://renovatebot.com/diffs/npm/metro-resolver/0.76.0/0.76.6)
|
[![age](https://badges.renovateapi.com/packages/npm/metro-resolver/0.76.6/age-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://badges.renovateapi.com/packages/npm/metro-resolver/0.76.6/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://badges.renovateapi.com/packages/npm/metro-resolver/0.76.6/compatibility-slim/0.76.0)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://badges.renovateapi.com/packages/npm/metro-resolver/0.76.6/confidence-slim/0.76.0)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>facebook/metro</summary>

###
[`v0.76.6`](https://togithub.com/facebook/metro/releases/tag/v0.76.6)

[Compare
Source](https://togithub.com/facebook/metro/compare/v0.76.5...v0.76.6)

- **\[Types]** Update config and `Server` types to use broader types
from `connect` package
(facebook/metro@d20d7c8
by [@&#8203;huntie](https://togithub.com/huntie))
- **\[Types]** Fix TypeScript name for `createConnectMiddleware` export
(facebook/metro@06682f8
by [@&#8203;huntie](https://togithub.com/huntie))
- **\[Deprecation]** Deprecate `server.enhanceMiddleware` option
(facebook/metro@22e85fd
by [@&#8203;huntie](https://togithub.com/huntie))

> NOTE: Experimental features are not covered by semver and can change
at any time.

- **\[Experimental]** Add `unstable_extraMiddleware` option to
`runServer` API
(facebook/metro@d0d5543
by [@&#8203;huntie](https://togithub.com/huntie))

**Full Changelog**:
facebook/metro@v0.76.5...v0.76.6

###
[`v0.76.5`](https://togithub.com/facebook/metro/releases/tag/v0.76.5)

[Compare
Source](https://togithub.com/facebook/metro/compare/v0.76.4...v0.76.5)

- **\[Feature]** Support URLs for both bundling and symbolication
requests using `//&` instead of `?` as a query string delimiter
(facebook/metro@bd357c8
by [@&#8203;robhogan](https://togithub.com/robhogan))
- **\[Fix]** Fix crash on a module added+modified+removed between
updates
(facebook/metro@5d7305e
by [@&#8203;robhogan](https://togithub.com/robhogan))
- **\[Fix]** Fix missed modification on module removed+modified+added
between updates
(facebook/metro@5d7305e
by [@&#8203;robhogan](https://togithub.com/robhogan))
- **\[Fix]** Emit source URLs in a format that will not be stripped by
JavaScriptCore
(facebook/metro@bce6b27ef8ac7c41e0a3e990eb71747cc0e6f606by
[@&#8203;robhogan](https://togithub.com/robhogan))
- **\[Performance]** Prune unmodified modules from delta updates before
sending them to the client
(facebook/metro@e24c6ae
by [@&#8203;robhogan](https://togithub.com/robhogan))

> NOTE: Experimental features are not covered by semver and can change
at any time.

- **\[Experimental]** Fix `babel/runtime` issue when using Package
Exports
(facebook/metro@905d773
by [@&#8203;huntie](https://togithub.com/huntie))

**Full Changelog**:
facebook/metro@v0.76.4...v0.76.5

###
[`v0.76.4`](https://togithub.com/facebook/metro/releases/tag/v0.76.4)

[Compare
Source](https://togithub.com/facebook/metro/compare/v0.76.3...v0.76.4)

- **\[Feature]**: Support the
[`x_google_ignoreList`](https://developer.chrome.com/articles/x-google-ignore-list/)
source map extension.
([facebook/metro#973,
[`82bd64a`](https://togithub.com/facebook/metro/commit/82bd64a9720174a9e2a02fb73bbef292153e20f1)
by [@&#8203;motiz88](https://togithub.com/motiz88))
- **\[Feature]**: Support bundling KTX files as image assets.
([facebook/metro#975
by [@&#8203;rshest](https://togithub.com/rshest))
- **\[Fix]**: Fix crash on a module added+modified+removed between
updates.
([`5d7305e`](https://togithub.com/facebook/metro/commit/5d7305e2f3a9f5f4aebc889a452afb03b1db12a7)
by [@&#8203;robhogan](https://togithub.com/robhogan))
- **\[Fix]**: Fix missed modification on module removed+modified+added
between updates.
([`5d7305e`](https://togithub.com/facebook/metro/commit/5d7305e2f3a9f5f4aebc889a452afb03b1db12a7)
by [@&#8203;robhogan](https://togithub.com/robhogan))

**Full Changelog**:
facebook/metro@v0.76.3...v0.76.4

###
[`v0.76.3`](https://togithub.com/facebook/metro/releases/tag/v0.76.3)

[Compare
Source](https://togithub.com/facebook/metro/compare/v0.76.2...v0.76.3)

- **\[Feature]**: Support custom `__loadBundleAsync` implementations in
the default `asyncRequire` function. See the [lazy bundling
RFC](https://togithub.com/react-native-community/discussions-and-proposals/blob/main/proposals/0605-lazy-bundling.md)
for more details.
(facebook/metro@ac3adce,
facebook/metro@f07ce5c
by [@&#8203;motiz88](https://togithub.com/motiz88))
- **\[Feature]**: Support `lazy` parameter in bundle requests. See the
[lazy bundling
RFC](https://togithub.com/react-native-community/discussions-and-proposals/blob/main/proposals/0605-lazy-bundling.md)
for more details.
(facebook/metro@4ef14f9
by [@&#8203;motiz88](https://togithub.com/motiz88))
- **\[Feature]**: Preserve comments in unminified builds, while
continuing to strip all comments from minified builds.
([facebook/metro#967
by [@&#8203;tido64](https://togithub.com/tido64))
- **\[Deprecated]**: The `transformer.asyncRequireModulePath` config
option is deprecated. Use
[`__loadBundleAsync`](https://togithub.com/react-native-community/discussions-and-proposals/blob/main/proposals/0605-lazy-bundling.md#\__loadbundleasync-in-metro)
instead.(facebook/metro@c7b684f
by [@&#8203;motiz88](https://togithub.com/motiz88))

> NOTE: Experimental features are not covered by semver and can change
at any time.

- **\[Experimental]** Package Exports unstable_conditionNames now
defaults to \['require', 'import']
(facebook/metro@e70ceef
by [@&#8203;huntie](https://togithub.com/huntie))
- **\[Experimental]** Removed `server.experimentalImportBundleSupport`
config option.
(facebook/metro@4ef14f9
by [@&#8203;motiz88](https://togithub.com/motiz88))

**Full Changelog**:
facebook/metro@v0.76.2...v0.76.3

###
[`v0.76.2`](https://togithub.com/facebook/metro/releases/tag/v0.76.2)

[Compare
Source](https://togithub.com/facebook/metro/compare/v0.76.1...v0.76.2)

- **\[Feature]**: Added customizeStack hook to Metro's `/symbolicate`
endpoint to allow custom frame skipping logic on a stack level.
(facebook/metro@ce266dd
by [@&#8203;GijsWeterings](https://togithub.com/GijsWeterings))
- **\[Feature]**: Re-export `metro-core`'s `Terminal` from `metro`.
(facebook/metro@86e3f93
by [@&#8203;robhogan](https://togithub.com/robhogan))
- **\[Feature]**: Re-export `metro-config`'s `resolveConfig` from
`metro`.
(facebook/metro@cc16664
by [@&#8203;robhogan](https://togithub.com/robhogan))
- **\[Types]**: Remove dependency on `@types/babel__code-frame`.
(facebook/metro@41cdc03
by [@&#8203;robhogan](https://togithub.com/robhogan))
- **\[Types]**: Remove dependency on `@types/ws`.
(facebook/metro@7deb525
by [@&#8203;robhogan](https://togithub.com/robhogan))
- **\[Types]**: Fix TypeScript types entry point for metro-source-map.
(facebook/metro@3238bbc
by [@&#8203;huntie](https://togithub.com/huntie))
- **\[Deprecated]**: Deprecate `ResolutionContext.getPackageForModule`.
(facebook/metro@2d0a01c
by [@&#8203;huntie](https://togithub.com/huntie))

> NOTE: Experimental features are not covered by semver and can change
at any time.

- **\[Experimental]**: Pass full path and query params to `asyncRequire`
for lazy bundles.
(facebook/metro@61a30b7
by [@&#8203;motiz88](https://togithub.com/motiz88))
- **\[Experimental]**: Fix bug where Package Exports warnings may have
been logged for nested `node_modules` path candidates.
(facebook/metro@29c77bf
by [@&#8203;huntie](https://togithub.com/huntie))
- **\[Experimental]**: Fix `package.json` discovery against root package
specifiers for Package Exports.
(facebook/metro@b995303
by [@&#8203;huntie](https://togithub.com/huntie), fixes
[facebook/metro#965
reported by [@&#8203;shamilovtim](https://togithub.com/shamilovtim))

**Full Changelog**:
facebook/metro@v0.76.1...v0.76.2

###
[`v0.76.1`](https://togithub.com/facebook/metro/releases/tag/v0.76.1)

[Compare
Source](https://togithub.com/facebook/metro/compare/v0.76.0...v0.76.1)

- **\[Feature]**: Support custom transformer/resolver options in `metro
build` and `runBuild` API.
(facebook/metro@fcfecc9
by [@&#8203;motiz88](https://togithub.com/motiz88))
- **\[Feature]**: `metro get-dependencies --entryFile <entryFile>` can
now be called as `metro get-dependencies <entryFile>`.
(facebook/metro@6fdce04
by [@&#8203;huntie](https://togithub.com/huntie))
- **\[Feature]**: Add `Content-Type` and `Content-Length` headers for
assets to Metro server.
([facebook/metro#953
by [@&#8203;aleqsio](https://togithub.com/aleqsio),
[facebook/metro#961
by [@&#8203;byCedric](https://togithub.com/byCedric))
- **\[Feature]**: Expose `mergeConfig` util from `metro` package.
(facebook/metro@aa8ec90
by [@&#8203;huntie](https://togithub.com/huntie))
- **\[Fix]**: `metro-file-map`: consistently abort crawl when `end()` is
called.
(facebook/metro@51877a8
by [@&#8203;motiz88](https://togithub.com/motiz88))
- **\[Fix]**: `metro-config`: Don't mutate argument to `loadConfig`.
(facebook/metro@38ec62d
by [@&#8203;motiz88](https://togithub.com/motiz88))
- **\[Fix]**: Babel transformers: Provide correct absolute source path
to plugins when Metro is not run from the project root.
(facebook/metro@de19bbd
by [@&#8203;robhogan](https://togithub.com/robhogan))
- **\[Fix]**:
[`resolver.assetExts`](https://facebook.github.io/metro/docs/configuration/#assetexts)
will now match asset files for extension values that include a dot
(`.`).
(facebook/metro@6d65a32
by [@&#8203;huntie](https://togithub.com/huntie))
- **\[Fix]**: Don't register an
[`unhandledRejection`](https://nodejs.org/api/process.html#event-unhandledrejection)
listener, fix spammy EventEmitter leak warning.
(facebook/metro@833f2ff
by [@&#8203;motiz88](https://togithub.com/motiz88))
- **\[Types]**: Add bundled TypeScript definitions (partial) for all
packages previously on DefinitelyTyped.
(facebook/metro@c022c36,
facebook/metro@07732e7,
facebook/metro@9ee5280
by [@&#8203;huntie](https://togithub.com/huntie), with
[@&#8203;afoxman](https://togithub.com/afoxman) and
[@&#8203;tido64](https://togithub.com/tido64))
- **\[Types]**: Expose `MetroConfig` type in `metro` package.
(facebook/metro@d2f3664
by [@&#8203;huntie](https://togithub.com/huntie))

> NOTE: Experimental features are not covered by semver and can change
at any time.

- **\[Experimental]**: Reorder `asyncRequire`'s parameters and make
module name optional.
(facebook/metro@4e5261c
by [@&#8203;motiz88](https://togithub.com/motiz88))
- **\[Experimental]**: Remove experimental `metro-hermes-compiler`
package.
(facebook/metro@833f2ff
by [@&#8203;motiz88](https://togithub.com/motiz88))
- **\[Experimental]**: Package Exports
[`unstable_conditionNames`](https://facebook.github.io/metro/docs/configuration/#unstable_conditionnames-experimental)
now defaults to `['require']`.
([facebook/metro#955
by [@&#8203;huntie](https://togithub.com/huntie))
- **\[Experimental]**: Add compatibility with legacy Node.js "exports"
array formats.
(facebook/metro@f321cff,
facebook/metro@1e47cb5
by [@&#8203;huntie](https://togithub.com/huntie))

**Full Changelog**:
facebook/metro@v0.76.0...v0.76.1

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about these
updates again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/levaintech/keychain).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS4xMzEuMCIsInVwZGF0ZWRJblZlciI6IjM1LjEzMS4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants