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 107 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

RedYetiDev · 2024-10-25T12:11:29Z

What happens if a non-file URL is passed? Does it throw an error, or an assertion?

JakobJingleheimer · 2024-10-25T13:44:55Z

What happens if a non-file URL is passed? Does it throw an error, or an assertion?

Yes, somewhere further along it complains when the scheme is disallowed.

nodejs-github-bot · 2024-10-25T13:49:22Z

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

nodejs-github-bot · 2024-10-25T20:40:56Z

Landed in e312d60

ShenHongFei · 2024-10-26T07:40:47Z

typings/internalBinding/modules.d.ts

  getNearestParentPackageJSONType(path: string): PackageConfig['type']
  getPackageScopeConfig(path: string): SerializedPackageConfig | undefined
  getPackageJSONScripts(): string | undefined
-  flushCompileCache(keepDeserializedCache?: boolean): void


why flushCompileCache is deleted ?

That's weird. I didn't delete it

PR-URL: #55412 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

Notable changes: 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: TODO

PR-URL: nodejs#55412 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

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 tools: * fix root certificate updater (Richard Lau) #55681 PR-URL: TODO

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

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

RedYetiDev added module Issues and PRs related to the module subsystem. and removed lib / src Issues and PRs related to general changes in the lib or src directory. labels Oct 25, 2024

aduh95 approved these changes Oct 25, 2024

View reviewed changes

JakobJingleheimer added the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 25, 2024

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 25, 2024

nodejs-github-bot merged commit e312d60 into nodejs:main Oct 25, 2024
63 checks passed

aduh95 mentioned this pull request Oct 25, 2024

module: simplify findPackageJSON implementation #55543

Merged

ShenHongFei reviewed Oct 26, 2024

View reviewed changes

JakobJingleheimer deleted the feat/module-utils-findPJSON branch October 28, 2024 12:50

RafaelGSS pushed a commit that referenced this pull request Nov 1, 2024

module: add findPackageJSON util

adda37f

PR-URL: #55412 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

louwers pushed a commit to louwers/node that referenced this pull request Nov 2, 2024

module: add findPackageJSON util

865a057

PR-URL: nodejs#55412 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

aduh95 mentioned this pull request Nov 6, 2024

2024-11-11, Version 23.2.0 (Current) #55741

Merged

JakobJingleheimer added backport-requested-v20.x PRs awaiting manual backport to the v20.x-staging branch. backport-requested-v22.x PRs awaiting manual backport to the v22.x-staging branch. labels Nov 8, 2024

JakobJingleheimer mentioned this pull request Nov 8, 2024

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

Merged

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

RedYetiDev commented Oct 25, 2024

JakobJingleheimer commented Oct 25, 2024

nodejs-github-bot commented Oct 25, 2024

nodejs-github-bot commented Oct 25, 2024

ShenHongFei Oct 26, 2024

JakobJingleheimer Oct 26, 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

RedYetiDev commented Oct 25, 2024

JakobJingleheimer commented Oct 25, 2024

nodejs-github-bot commented Oct 25, 2024

nodejs-github-bot commented Oct 25, 2024

ShenHongFei Oct 26, 2024

Choose a reason for hiding this comment

JakobJingleheimer Oct 26, 2024

Choose a reason for hiding this comment

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