module: avoid throwing when findPackageJSON is called on nonexistent

[JakobJingleheimer]

The edge-case occurs when ESMLoader can't resolve the target.

The documented behaviour is already for it to return undefined, so this fixes a small bug.

[nodejs-github-bot]

Review requested:

• @nodejs/loaders

[aduh95]

The commit message guidelines stipulates that the word after the subsystem needs to be an imperative verb, could you amend it please?

[JakobJingleheimer]

@aduh95 the character limit makes this quite a challenge 😅

[aduh95]

Here's a suggestion: module: avoid throwing when `findPackageJSON` is called on inexistant

[anonrig]

Can you add a test?

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.41%. Comparing base (32ff100) to head (c02aebc).
Report is 355 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #55822 +/- ##
==========================================
+ Coverage 88.39% 88.41% +0.01% 
==========================================
Files 654 654 Lines 187560 187814 +254 Branches 36087 36132 +45 ==========================================
+ Hits 165800 166048 +248 - Misses 14996 15007 +11 + Partials 6764 6759 -5 

Files with missing lines	Coverage Δ
lib/internal/modules/package_json_reader.js	99.39% <100.00%> (+<0.01%)	⬆️

... and 49 files with indirect coverage changes

[JakobJingleheimer]

Can you add a test?

Yes, I intend to do, but I have to re-figure out how to re-produce it. I'm 100% certain this fixes the issue and does not cause adverse results, so can we land this without the test-case and add it subsequently so users can get the fix rather than wait?

[JakobJingleheimer]

@aduh95 now that I re-read the updated title, I think it's actually not accurate: the issue does not occur when findPackageJSON(nonexistent), but rather when ESMLoader can't resolve the target, which is subtly different.

[aduh95]

so can we land this without the test-case and add it subsequently

Let’s not, without a test we cannot detect regressions.

[JakobJingleheimer]

@AugustinMauroy found the reproduction 🎉 https://github.com/AugustinMauroy/issue-node-findpackagejson

[ljharb]

fix LGTM but i agree this should probably have a test case before landing