Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

module: add findNearestPackageJSON utility #55173

Conversation

JakobJingleheimer
Copy link
Member

This draft is not ready for review. Opening it in its current state for visibility.

Split from #54992

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/loaders

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. module Issues and PRs related to the module subsystem. needs-ci PRs that need a full CI run. typings labels Sep 30, 2024
@JakobJingleheimer
Copy link
Member Author

JakobJingleheimer commented Sep 30, 2024

@anonrig what is the difference between GetNearestParentPackageJSON and GetPackageScopeConfig?

/**
* Find the nearest package.json
* @param {URL['pathname']} origin Where to start searching.
* @returns {URL['pathname']} The fully resolved location of the package.json file.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: Do we need to also make sure this returns a UNC path on Windows? UNC path (the result of toNamespacedPath() is required if the path is extremely long. windows weirdly throws an error if long path feature is not enabled)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @returns {URL['pathname']} The fully resolved location of the package.json file.
* @returns {URL['pathname'] | undefined} The fully resolved location of the package.json file.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding your question: i have no idea. It sounds like you answered it yourself though (and the answer is "yes").

void BindingData::GetNearestParentPackageJSON(
const v8::FunctionCallbackInfo<v8::Value>& args) {
void BindingData::FindNearestParentPackageJSON(
const v8::FunctionCallbackInfo<v8::Value>& args
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change seems to be unrelated

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically yes, this un-mangling is not required to make the change work.

@anonrig
Copy link
Member

anonrig commented Sep 30, 2024

@anonrig what is the difference between GetNearestParentPackageJSON and c?

GetNearestParentPackageJSON:

  • Traverses through the root of the current file until it finds a package.json file and returns it.

GetNearestParentPackageJSON

  • Does the same thing but accepts a boolean flag to add slash.

@JakobJingleheimer
Copy link
Member Author

This was superfluous 😞

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. module Issues and PRs related to the module subsystem. needs-ci PRs that need a full CI run. typings
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants