module: add `findPackageJSON` util #55412

JakobJingleheimer · 2024-10-16T20:54:49Z

#55229 was too controversial, so back to the original idea from #55173.

nodejs-github-bot · 2024-10-16T20:54:54Z

Review requested:

@nodejs/loaders

ljharb · 2024-10-16T21:18:21Z

so is this basically just https://www.npmjs.com/package/find-pkg / https://www.npmjs.com/package/pkg-up / https://www.npmjs.com/package/package-up ?

JakobJingleheimer · 2024-10-16T21:37:18Z

"Just" seems a little dismissive 😕

The first package was last updated 7 years ago (so it may well be outdated, plus its cited limitations).

The second package is deprecated.

The third is maybe similar/close; it appears to only do direct path manipulation and won't respect module resolution (ex from a loader hook), which this does respect.

Edit: looking closer, the third package has several limitations that this does not, such how it handles relative search (cwd vs current module location) and, AFAICS, it does not handle node_modules.

This package is also leveraging/mirroring node's internals, so it will behave exactly as node is doing internally (whereas userland may not, and would have to re-invent the wheel to do so).

ljharb · 2024-10-16T21:39:58Z

apologies, you're right, a better way to phrase it is "is this the same capability as these packages".

Certainly doing this internally in node is better for tons of reasons, and I'm glad you're adding it :-) I wanted to confirm what the semantics were, is all.

JakobJingleheimer · 2024-10-16T21:44:35Z

I wanted to confirm what the semantics were, is all.

Ah, yes then 🙂

JakobJingleheimer · 2024-10-16T21:48:04Z

The markdown linter is complaining about a codeblock that has no language. It's a directory listing—language is not applicable (it just needs the default codeblock treatment).

Can I disable the rule for that block?

ljharb · 2024-10-16T21:50:26Z

can you tag it as sh?

codecov · 2024-10-16T22:09:22Z

Codecov Report

Attention: Patch coverage is 98.80478% with 3 lines in your changes missing coverage. Please review.

Project coverage is 88.41%. Comparing base (53b1050) to head (ba9acf9).
Report is 237 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/internal/modules/package_json_reader.js	99.08%	2 Missing ⚠️
lib/internal/modules/cjs/loader.js	90.90%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #55412      +/-   ##
==========================================
- Coverage   88.42%   88.41%   -0.01%     
==========================================
  Files         654      654              
  Lines      187552   187657     +105     
  Branches    36087    36117      +30     
==========================================
+ Hits       165839   165919      +80     
- Misses      14950    14972      +22     
- Partials     6763     6766       +3

Files with missing lines	Coverage Δ
lib/internal/modules/esm/resolve.js	`96.17% <100.00%> (-0.16%)`	⬇️
lib/module.js	`100.00% <100.00%> (ø)`
lib/internal/modules/cjs/loader.js	`97.59% <90.90%> (-0.06%)`	⬇️
lib/internal/modules/package_json_reader.js	`99.40% <99.08%> (-0.60%)`	⬇️

... and 34 files with indirect coverage changes

JakobJingleheimer · 2024-10-17T07:13:13Z

can you tag it as sh?

That could hack around it, but it would enable shell syntax highlighting. There should be no syntax highlighting.

targos · 2024-10-17T07:19:20Z

What about text? It's already used in many places of the docs.

JakobJingleheimer · 2024-10-17T07:29:25Z

What about text? It's already used in many places of the docs.

Huzzah! perfect. I didn't know that was an option. Thanks!

doc/api/module.md

lib/internal/modules/package_json_reader.js

doc/api/module.md

test/parallel/test-find-package-json.js

doc/api/module.md

mcollina

lgtm

nodejs-github-bot · 2024-10-17T21:45:24Z

CI: https://ci.nodejs.org/job/node-test-pull-request/63167/

test/parallel/test-find-package-json.js

Notable changes: crypto: * update root certificates to NSS 3.104 (Richard Lau) #55681 doc: * move typescript support to active development (Marco Ippolito) #55536 * add jazelly to collaborators (Jason Zhang) #55531 fs: * (SEMVER-MINOR) make `dirent.path` writable (Antoine du Hamel) #55547 http: * (SEMVER-MINOR) add diagnostic channel `http.client.request.created` (Marco Ippolito) #55586 module: * (SEMVER-MINOR) add `findPackageJSON` util (Jacob Smith) #55412 * (SEMVER-MINOR) add `module.stripTypeScriptTypes` (Marco Ippolito) #55282 PR-URL: #55741

PR-URL: nodejs#55412 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>

Notable changes: crypto: * update root certificates to NSS 3.104 (Richard Lau) nodejs#55681 doc: * move typescript support to active development (Marco Ippolito) nodejs#55536 * add jazelly to collaborators (Jason Zhang) nodejs#55531 fs: * (SEMVER-MINOR) make `dirent.path` writable (Antoine du Hamel) nodejs#55547 http: * (SEMVER-MINOR) add diagnostic channel `http.client.request.created` (Marco Ippolito) nodejs#55586 module: * (SEMVER-MINOR) add `findPackageJSON` util (Jacob Smith) nodejs#55412 * (SEMVER-MINOR) add `module.stripTypeScriptTypes` (Marco Ippolito) nodejs#55282 PR-URL: nodejs#55741

ruyadorno · 2024-11-27T06:38:46Z

This commit does not land cleanly on v22.x-staging and will need manual backport in case we want it in v22.x.

JakobJingleheimer · 2024-11-29T12:26:42Z

@ruyadorno is this something I can help with? Would be great to get this in the pending 22.x release.

PS sorry I didn't get a notification for your message.

aduh95 · 2024-11-29T12:30:07Z

@JakobJingleheimer yes, you can open a backport PR that cherry-picks adda37f on top of v22.x-staging

PR-URL: nodejs#55412 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>

PR-URL: nodejs#55412 Backport-PR-URL: nodejs#56074 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>

PR-URL: nodejs#55543 Backport-PR-URL: nodejs#56074 Reviewed-By: Jacob Smith <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> PR-URL: nodejs#55412

Notable changes: assert: * (SEMVER-MINOR) add partialDeepStrictEqual (Giovanni Bucci) #54630 cli: * (SEMVER-MINOR) implement --trace-env and --trace-env-[js|native]-stack (Joyee Cheung) #55604 crypto: * graduate WebCryptoAPI Ed25519 and X25519 algorithms as stable (Filip Skokan) #56142 dgram: * (SEMVER-MINOR) support blocklist in udp (theanarkh) #56087 doc: * stabilize util.styleText (Rafael Gonzaga) #56265 * move typescript support to active development (Marco Ippolito) #55536 * add LJHarb to collaborators (Jordan Harband) #56132 * (SEMVER-MINOR) add report version and history section (Chengzhong Wu) #55697 * (SEMVER-MINOR) sort --report-exclude alphabetically (Rafael Gonzaga) #55697 doc,lib,src,test: * (SEMVER-MINOR) unflag sqlite module (Colin Ihrig) #55890 module: * (SEMVER-MINOR) only emit require(esm) warning under --trace-require-module (Joyee Cheung) #56194 * (SEMVER-MINOR) add module.stripTypeScriptTypes (Marco Ippolito) #55282 * (SEMVER-MINOR) simplify `findPackageJSON` implementation (Antoine du Hamel) #55412 * (SEMVER-MINOR) add `findPackageJSON` util (Jacob Smith) #55412 net: * (SEMVER-MINOR) support blocklist in net.connect (theanarkh) #56075 * (SEMVER-MINOR) add SocketAddress.parse (James M Snell) #56076 * (SEMVER-MINOR) add net.BlockList.isBlockList(value) (James M Snell) #56078 * (SEMVER-MINOR) support blocklist for net.Server (theanarkh) #56079 process: * (SEMVER-MINOR) deprecate `features.{ipv6,uv}` and `features.tls_*` (René) #55545 report: * (SEMVER-MINOR) fix typos in report keys and bump the version (Yuan-Ming Hsu) #56068 sqlite: * (SEMVER-MINOR) aggregate constants in a single property (Edigleysson Silva (Edy)) #56213 * (SEMVER-MINOR) add `StatementSync.prototype.iterate` method (tpoisseau) #54213 src: * (SEMVER-MINOR) add cli option to preserve env vars on dr (Rafael Gonzaga) #55697 src,lib: * (SEMVER-MINOR) stabilize permission model (Rafael Gonzaga) #56201 util: * (SEMVER-MINOR) add sourcemap support to getCallSites (Marco Ippolito) #55589 PR-URL: TODO

Notable changes: assert: * (SEMVER-MINOR) add partialDeepStrictEqual (Giovanni Bucci) #54630 cli: * (SEMVER-MINOR) implement --trace-env and --trace-env-[js|native]-stack (Joyee Cheung) #55604 crypto: * graduate WebCryptoAPI Ed25519 and X25519 algorithms as stable (Filip Skokan) #56142 dgram: * (SEMVER-MINOR) support blocklist in udp (theanarkh) #56087 doc: * stabilize util.styleText (Rafael Gonzaga) #56265 * move typescript support to active development (Marco Ippolito) #55536 * add LJHarb to collaborators (Jordan Harband) #56132 * (SEMVER-MINOR) add report version and history section (Chengzhong Wu) #55697 * (SEMVER-MINOR) sort --report-exclude alphabetically (Rafael Gonzaga) #55697 doc,lib,src,test: * (SEMVER-MINOR) unflag sqlite module (Colin Ihrig) #55890 module: * (SEMVER-MINOR) only emit require(esm) warning under --trace-require-module (Joyee Cheung) #56194 * (SEMVER-MINOR) add module.stripTypeScriptTypes (Marco Ippolito) #55282 * (SEMVER-MINOR) simplify `findPackageJSON` implementation (Antoine du Hamel) #55412 * (SEMVER-MINOR) add `findPackageJSON` util (Jacob Smith) #55412 net: * (SEMVER-MINOR) support blocklist in net.connect (theanarkh) #56075 * (SEMVER-MINOR) add SocketAddress.parse (James M Snell) #56076 * (SEMVER-MINOR) add net.BlockList.isBlockList(value) (James M Snell) #56078 * (SEMVER-MINOR) support blocklist for net.Server (theanarkh) #56079 process: * (SEMVER-MINOR) deprecate `features.{ipv6,uv}` and `features.tls_*` (René) #55545 report: * (SEMVER-MINOR) fix typos in report keys and bump the version (Yuan-Ming Hsu) #56068 sqlite: * (SEMVER-MINOR) aggregate constants in a single property (Edigleysson Silva (Edy)) #56213 * (SEMVER-MINOR) add `StatementSync.prototype.iterate` method (tpoisseau) #54213 src: * (SEMVER-MINOR) add cli option to preserve env vars on dr (Rafael Gonzaga) #55697 src,lib: * (SEMVER-MINOR) stabilize permission model (Rafael Gonzaga) #56201 util: * (SEMVER-MINOR) add sourcemap support to getCallSites (Marco Ippolito) #55589 PR-URL: #56329

JakobJingleheimer added the semver-minor PRs that contain new features and should be released in the next minor version. label Oct 16, 2024

JakobJingleheimer requested review from aduh95 and marco-ippolito October 16, 2024 20:54

nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Oct 16, 2024

JakobJingleheimer added the commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. label Oct 16, 2024

aduh95 reviewed Oct 17, 2024

View reviewed changes

test/parallel/test-find-package-json.js Outdated Show resolved Hide resolved

JakobJingleheimer commented Oct 17, 2024

View reviewed changes

test/parallel/test-find-package-json.js Outdated Show resolved Hide resolved

marco-ippolito reviewed Oct 17, 2024

View reviewed changes

doc/api/module.md Outdated Show resolved Hide resolved

mcollina approved these changes Oct 17, 2024

View reviewed changes

JakobJingleheimer requested review from marco-ippolito and aduh95 October 17, 2024 16:45

JakobJingleheimer added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 17, 2024

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 17, 2024

aduh95 reviewed Oct 17, 2024

View reviewed changes

test/parallel/test-find-package-json.js Outdated Show resolved Hide resolved

aduh95 reviewed Oct 17, 2024

View reviewed changes

test/parallel/test-find-package-json.js Outdated Show resolved Hide resolved

aduh95 reviewed Oct 17, 2024

View reviewed changes

test/parallel/test-find-package-json.js Outdated Show resolved Hide resolved

JakobJingleheimer mentioned this pull request Nov 8, 2024

chore(monorepo): separate each loader into its own package nodejs-loaders/nodejs-loaders#23

Merged

tpoisseau pushed a commit to tpoisseau/node that referenced this pull request Nov 21, 2024

module: add findPackageJSON util

787783a

PR-URL: nodejs#55412 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>

ruyadorno added the backport-requested-v22.x PRs awaiting manual backport to the v22.x-staging branch. label Nov 27, 2024

JakobJingleheimer mentioned this pull request Nov 29, 2024

2024-12-03, Version 22.12.0 'Jod' (LTS) #56040

Closed

JakobJingleheimer mentioned this pull request Nov 29, 2024

[v22.x backport] module: add findPackageJSON util #56074

Merged

JakobJingleheimer added a commit to JakobJingleheimer/node that referenced this pull request Nov 29, 2024

module: add findPackageJSON util

f02a8ae

PR-URL: nodejs#55412 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>

aduh95 added backport-open-v22.x Indicate that the PR has an open backport and removed backport-requested-v22.x PRs awaiting manual backport to the v22.x-staging branch. lts-watch-v22.x PRs that may need to be released in v22.x labels Nov 29, 2024

aduh95 pushed a commit to JakobJingleheimer/node that referenced this pull request Dec 10, 2024

module: add findPackageJSON util

807f34f

PR-URL: nodejs#55412 Backport-PR-URL: nodejs#56074 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>

ruyadorno mentioned this pull request Dec 20, 2024

2025-01-07, Version 22.13.0 'Jod' (LTS) #56329

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

module: add `findPackageJSON` util #55412

module: add `findPackageJSON` util #55412

JakobJingleheimer commented Oct 16, 2024

nodejs-github-bot commented Oct 16, 2024

ljharb commented Oct 16, 2024

JakobJingleheimer commented Oct 16, 2024 •

edited

Loading

ljharb commented Oct 16, 2024

JakobJingleheimer commented Oct 16, 2024

JakobJingleheimer commented Oct 16, 2024

ljharb commented Oct 16, 2024

codecov bot commented Oct 16, 2024 •

edited

Loading

JakobJingleheimer commented Oct 17, 2024

targos commented Oct 17, 2024

JakobJingleheimer commented Oct 17, 2024

mcollina left a comment

nodejs-github-bot commented Oct 17, 2024

ruyadorno commented Nov 27, 2024

JakobJingleheimer commented Nov 29, 2024

aduh95 commented Nov 29, 2024

module: add findPackageJSON util #55412

module: add findPackageJSON util #55412

Conversation

JakobJingleheimer commented Oct 16, 2024

nodejs-github-bot commented Oct 16, 2024

ljharb commented Oct 16, 2024

JakobJingleheimer commented Oct 16, 2024 • edited Loading

ljharb commented Oct 16, 2024

JakobJingleheimer commented Oct 16, 2024

JakobJingleheimer commented Oct 16, 2024

ljharb commented Oct 16, 2024

codecov bot commented Oct 16, 2024 • edited Loading

Codecov Report

JakobJingleheimer commented Oct 17, 2024

targos commented Oct 17, 2024

JakobJingleheimer commented Oct 17, 2024

mcollina left a comment

Choose a reason for hiding this comment

nodejs-github-bot commented Oct 17, 2024

ruyadorno commented Nov 27, 2024

JakobJingleheimer commented Nov 29, 2024

aduh95 commented Nov 29, 2024

module: add `findPackageJSON` util #55412

module: add `findPackageJSON` util #55412

JakobJingleheimer commented Oct 16, 2024 •

edited

Loading

codecov bot commented Oct 16, 2024 •

edited

Loading