Resolve aliased module from closest package directory

[SlyryD]

Summary

Fixes #860 by resolving realModuleName from the closest package directory.

Example package.json: { "name": "@foo/bar", "browser": { "util": "./lib/polyfills/util.js" } }

Example values: originModulePath := "/path/to/node_modules/@foo/bar/lib/index.js" realModuleName := "./lib/polyfills/util.js"

Before (wrong originModuleDir)

originModuleDir := "/path/to/node_modules/@foo" absPath := "/path/to/node_modules/@foo/lib/polyfills/util.js"

After (correct originModuleDir)

originModuleDir := "/path/to/node_modules/@foo/bar" absPath := "/path/to/node_modules/@foo/bar/lib/polyfills/util.js"

Test plan

TBD

[facebook-github-bot]

Hi @SlyryD!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

[SlyryD]

@rafeca any feedback here? Looks like the original code went in 5 years ago, so it's interesting I'm the first person to hit it.

[robhogan]

Hi @SlyryD, thanks for the report and PR - this is a little surprising because afaik scoped packages have been fully supported for some time. I'll take a look tomorrow but might ask for a repro case.

[SlyryD]

Hi @robhogan, thanks for the response! Admittedly I'm having trouble reproing it outside the private repo where I see the issue (which has a different package manager than yarn). Here's my attempt to repro the issue:

1. Clone https://github.com/SlyryD/metro-repro
2. Run yarn at root
3. Run yarn run bundle at packages/baz

I'm getting a different issue currently, which I think has to do with "@rnx-kit/metro-resolver-symlinks". I'm starting to think that could be the problematic package, though this PR does fix issues in the private repo.

[SlyryD]

Hi @robhogan, thanks for the response! Admittedly I'm having trouble reproing it outside the private repo where I see the issue (which has a different package manager than yarn). Here's my attempt to repro the issue:

1. Clone https://github.com/SlyryD/metro-repro
2. Run yarn at root
3. Run yarn run bundle at packages/baz

I'm getting a different issue currently, which I think has to do with "@rnx-kit/metro-resolver-symlinks". I'm starting to think that could be the problematic package, though this PR does fix issues in the private repo.

I got a repro, but it's a little bit different because it doesn't have to do with scoped packages. That being said, this PR fixes the issue too:

1. Clone https://github.com/SlyryD/metro-repro
2. Run yarn at root (in main branch)
3. Run yarn run bundle at packages/car

[SlyryD]

Hi @robhogan, thanks for the response! Admittedly I'm having trouble reproing it outside the private repo where I see the issue (which has a different package manager than yarn). Here's my attempt to repro the issue:

1. Clone https://github.com/SlyryD/metro-repro
2. Run yarn at root
3. Run yarn run bundle at packages/baz

I'm getting a different issue currently, which I think has to do with "@rnx-kit/metro-resolver-symlinks". I'm starting to think that could be the problematic package, though this PR does fix issues in the private repo.

I got a repro, but it's a little bit different because it doesn't have to do with scoped packages. That being said, this PR fixes the issue too:

1. Clone https://github.com/SlyryD/metro-repro
2. Run yarn at root (in main branch)
3. Run yarn run bundle at packages/car

Any update now that I've provided a repro

[SlyryD]

I provided a repro for an issue above. Any updates? Thanks!

[robhogan]

Hi @SlyryD - I just checked out the repro.

"browser": { "util": "./lib/polyfills/util.js" }

This isn't standard node_modules resolution, is it? It looks like https://github.com/defunctzombie/package-browser-field-spec#replace-specific-files---advanced , which Metro doesn't support AFAIK.

The PR is also makes some incorrect assumptions about package layout - the package.json isn't necessarily in the parent directory of the requiring module and it looks like this would break with nesting, or indeed when the module is in the same directory as the package.json.

If the issue only appears when using the rnx-kit resolver, I'd start by filing an issue over there, or otherwise reduce the repro to something that demonstrates Metro's resolveRequest is behaving incorrectly. If you can do that, please do reopen an issue and we'll work from there.

I'm going to close the PR as we need to concretely identify an issue and then work out the best fix that doesn't break other cases - thanks for your effort though.