fix(resolve)!: better search path normalization

[pi0]

resolves #158

Due to legacy reasons, resolve utils were wrongly trying to search outside of the current dir.

This PR improves behavior and fixes situations where no URL was passed or URL is absolute path to directory without file://. (check diff for details)

Note: I just learned that Node.js actually has different behavior for file: URLs with trailing slash, resolving from file:///Users/pooya/Code/mlly/ is the same as resolving from file:///Users/pooya/Code/mlly/_index.js.

Behavior comparison

1. default (no url option) [fixed]

• Before:
  • file:///Users/pooya/Code/mlly (wrong)
  • file:///Users/pooya/Code/ (wrong)
  • file:///Users/pooya/Code/mlly/_index.js (correct)
  • file:///Users/pooya/Code/node_modules (wrong)
• After:
  • file:///Users/pooya/Code/mlly/ (correct)

2. Passing file:// URL (import.meta.url) [less fallbacks]

• Before:
  • file:///Users/pooya/Code/mlly/test.mjs (correct)
  • file:///Users/pooya/Code/mlly/ (correct)
  • file:///Users/pooya/Code/mlly/test.mjs/_index.js (correct)
  • file:///Users/pooya/Code/mlly/node_modules (correct)
• After:
  • file:///Users/pooya/Code/mlly/test.mjs (correct)

3. Passing file path (__filename) [less fallbacks]

• Before:
  • file:///Users/pooya/Code/mlly/test.mjs (correct)
  • file:///Users/pooya/Code/mlly/ (correct)
  • file:///Users/pooya/Code/mlly/test.mjs/_index.js (correct)
  • file:///Users/pooya/Code/mlly/node_modules (correct)
• After:
  • file:///Users/pooya/Code/mlly/test.mjs (correct) [costs statSync()]

4. Passing dir path (__dirname or cwd()) [fixed]

• Before:
  • file:///Users/pooya/Code/mlly (wrong)
  • file:///Users/pooya/Code/ (wrong)
  • file:///Users/pooya/Code/mlly/_index.js (correct)
  • file:///Users/pooya/Code/node_modules (wrong)
• After:
  • file:///Users/pooya/Code/mlly/ (correct) [costs statSync()]

[antfu]

I'd love to see the bug get fixed and consistency improve. While I did not encounter issues regarding this, it's a bit hard for me to verify if dependents of this would be affected or not from looking at the code. Do you consider introducing http://pkg.pr.new/ where we could test this with variety of projects to check the affecting surface before landing?

[danielroe]

Reading through, the logic looks good to me. 👌 More than happy to do some testing in real projects but won't have time until tonight, but don't let me stop the merge.

[pi0]

Thanks for reviewing!

I will probably introduce change as v2 beta tag and backport later to v1 if we haven’t seen any issues in wild (however thinking more could be still tricky if some usages was depending on incorrect parent resolution)

[danielroe]

oh, I wish v2 would be released with this fix 😆 (it's so good!)

if we need more testing (understandable) would you be up for a perf improvement PR in v1 branch? For example, there's currently no deduplication if a URL is passed that matches one of the URLs that would be created, and we have URL -> normalisation -> URL behaviour which seems to be a negative from perf point of view....

[pi0]

Good point for deduplicate let me see if can fit it in (it is so common and worth to test for v2, we add nested node_modules as search path, only deepest with common base is needed)

Only tricky point is, we should preserve order..