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

fix(pkg): display if any of multiple attributes exist #7855

Merged
merged 1 commit into from
Oct 18, 2024

Conversation

Sanderovich
Copy link
Contributor

See #7854 for the why and more details.

I changed the unwrap logic so that it only runs when the result contains one entry and the amount of arguments is one. Otherwise, when multiple arguments are provided the unwrap results in an undefined. Additionally, I think it makes sense to skip the unwrap logic when the result contains one entry and pkg has multiple arguments because you would expect an object.

In the existing comment above the if-statement, it is mentioned the CLI should not unwrap at all. I chose not to do this because it is not clear to me if this is a final decision and has a large impact. However, if it is the desired direction, I am happy to do the work.

References

Fixes #7854

@Sanderovich Sanderovich requested a review from a team as a code owner October 18, 2024 12:56
@wraithgar wraithgar changed the title Fix npm pkg get does not return result when only one of multiple arguments exist in the package.json fix(pkg): display if any of multiple attributes exist Oct 18, 2024
@wraithgar
Copy link
Member

Additionally, I think it makes sense to skip the unwrap logic when the result contains one entry and pkg has multiple arguments because you would expect an object

100%, if the user asked for more than one attribute they're always getting an object back.

@wraithgar
Copy link
Member

It may be time to address that breaking change TODO comment. The timing is good cause we're building npm 11 right now. What do you think the logic should be here, then?

@wraithgar
Copy link
Member

This PR is good, and done, and shouldn't be blocked by discussion of a potential breaking change. We can move forward discussing that still, but this PR is shipping now. Thank you.

@wraithgar wraithgar merged commit 780afc5 into npm:latest Oct 18, 2024
19 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] npm pkg get does not return result when only one of multiple arguments exist in the package.json
2 participants