| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
Review requested:
|
Sorry, something went wrong.
|
The commit message guidelines stipulates that the word after the subsystem needs to be an imperative verb, could you amend it please? |
Sorry, something went wrong.
|
@aduh95 the character limit makes this quite a challenge 😅 |
Sorry, something went wrong.
|
Here's a suggestion: module: avoid throwing when `findPackageJSON` is called on inexistant |
Sorry, something went wrong.
There was a problem hiding this comment.
Can you add a test?
Sorry, something went wrong.
Codecov ReportAll modified and coverable lines are covered by tests ✅ 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
|
Sorry, something went wrong.
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? |
Sorry, something went wrong.
|
@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. |
Sorry, something went wrong.
Let’s not, without a test we cannot detect regressions. |
Sorry, something went wrong.
|
@AugustinMauroy found the reproduction 🎉 https://github.com/AugustinMauroy/issue-node-findpackagejson |
Sorry, something went wrong.
|
fix LGTM but i agree this should probably have a test case before landing |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
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.