Skip to content

Commit

Permalink
Fix getPackageForModule implementation against a package root path
Browse files Browse the repository at this point in the history
Summary:
Fixes #965.

**Cause**: The implementation of `context.getPackageForModule(modulePath: string)` attempts to locate a `package.json` file starting with `parsedPath.dir` — however this clobbers the path basename when `modulePath` is initially the package directory.

e.g. for the path `/root/node_modules/ipld/dag-cbor` (a package root):

- `/root/node_modules/ipld/dag-cbor/package.json` **exists**.
- `getPackageForModule('/root/node_modules/ipld/dag-cbor')` begins traversal, **incorrectly**, from `/root/node_modules/ipld`.

https://pxl.cl/2Cb1T

**Why this didn't matter until now**

- This was previously inconsequential for `mainFields` (`"browser"` field spec), since redirections ***only*** support subpaths. (`"browser"` does not describe/permit a `"."` subpath, but instead a redirection of a *file path*, e.g. `"./index.js"`.)
https://github.com/defunctzombie/package-browser-field-spec#replace-specific-files---advanced

- On fall through from `mainFields` subpath resolution (implemented via `context.redirectModulePath` and not sharing the regular `nodeModulesPath` iteration loop), `resolveModulePath` will attempt `./package.json` to call [`getPackageEntryPoint`](https://github.com/facebook/metro/blob/d0b8ea57749d081f66c62ad4a4863caff62aa1d4/packages/metro-resolver/src/PackageResolve.js#L23).

Note: In tests, this behaviour was already (necessarily) added in D43055160, but did not line up!:
https://github.com/facebook/metro/blob/d0b8ea57749d081f66c62ad4a4863caff62aa1d4/packages/metro-resolver/src/__tests__/utils.js#L102

Changelog: **[Experimental]** Fix `package.json` discovery against root package specifiers for Package Exports

(⬆️ Would previously fall-through to file-based resolution.)

Reviewed By: robhogan

Differential Revision: D44877146

fbshipit-source-id: bb02d81b9e7d2ef71846d492cfbc38c85a4bf98b
  • Loading branch information
huntie authored and facebook-github-bot committed Apr 13, 2023
1 parent fdd9a1e commit b995303
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 2 deletions.
2 changes: 1 addition & 1 deletion packages/metro-resolver/src/__tests__/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ export function createPackageAccessors(
const getPackageForModule = (modulePath: string) => {
const parsedPath = path.parse(modulePath);
const root = parsedPath.root;
let dir = path.join(parsedPath.dir, parsedPath.name);
let dir = path.join(parsedPath.dir, parsedPath.base);

do {
if (path.basename(dir) === 'node_modules') {
Expand Down
3 changes: 2 additions & 1 deletion packages/metro/src/node-haste/DependencyGraph.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,8 @@ class DependencyGraph extends EventEmitter {
_getClosestPackage(filePath: string): ?string {
const parsedPath = path.parse(filePath);
const root = parsedPath.root;
let dir = parsedPath.dir;
let dir = path.join(parsedPath.dir, parsedPath.base);

do {
// If we've hit a node_modules directory, the closest package was not
// found (`filePath` was likely nonexistent).
Expand Down

0 comments on commit b995303

Please sign in to comment.