fix: correctly resolve remapped directories

[SimenB]

Summary

Old OP

For now, just a failing test.

The issue is that resolve tries to find a directory, and when there's a mapping, that doesn't exist. Not sure how, but resolve needs to find the closest package.json first, load it, pass it through filters, then try to request from disk. Or something like it - we need to map the request before resolve checks if it's on disk. Currently neither packageFilter nor pathFilter is called before resolves throws MODULE_NOT_FOUND.

https://github.com/browserify/resolve/blob/8641b137f5fc1c4d40164c3aae7f4e492c332e42/lib/sync.js#L207-L220

/cc @ljharb any ideas on best approach here?

Rewrites the logic that respects exports to

1. check if it's a module that's being required (i.e. path does not start with . or is absolute)
2. load its package.json from root using resolve so paths etc is respected
3. if exports exist, resolve it and return the absolute path
4. if any of the above checks bail, proceed with a "normal" resolve lookup, like we had before feat: support . in exports field #11919 and feat: support exports in package.json #11961

Fixes #12372

Test plan

Green CI (eventually)

[ljharb]

I'm pretty unclear on the problem. When you say "remapped", do you mean like, jest config that's trying to map one folder path to another?

[SimenB]

When exports maps from a non-existing directory to an existent one, like: https://github.com/facebook/jest/pull/12373/files#diff-19fd284b5181b406231ab6bd894afeeabe5afa1020dc19f1199f65154f58c038R15

(Unsure if link works, but look at diff in package.json of this PR)

[ljharb]

ahh, right. yeah this is one of the bigger stumbling blocks i've ran into for resolve's implementation :-/ i don't have any easy solutions

[SimenB]

Right, none of the hooks (except isDirectory) was called. I'll play with walking up file tree next week

[SimenB]

From looking at https://nodejs.org/api/modules.html#all-together, and specifically LOAD_PACKAGE_EXPORTS, it seems we should only read the root package.json of the module (at least the others specify "closest" while this does not), meaning there should be no traversal like my original plan was. So I'll try to split up the import specifier before calling resolve and do the mapping then (if exports exists), before passing the already mapped specifier back to resolve. Might actually simplify the code somewhat

[SimenB]

Have a somewhat working version of that now, but it fails when paths is supplied, which probably makes sense. So might have to do this inside resolve after all

[SimenB]

pushed up my half-working code, if by chance somebody comes over this and gives it a try before I get back to it myself 😀

[SimenB]

I honestly don't understand this test 🙈 Comes from #1572

[SimenB]

comment above doesn't make much sense, you're not simulating a real environment by using a relative path to get into some module

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.