Skip to content

feat: config get prints json for object #9811

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

Draft
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

KSXGitHub
Copy link
Contributor

Resolves #9797

return { output: Array.isArray(config) ? config.join(',') : String(config), exitCode: 0 }
let output: string
if (Array.isArray(config)) {
output = config.join(',')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was on me: #7917. I don't remember why I decided to return a comma-separated list instead of a JSON string, but changing it now would be a breaking change.

@KSXGitHub
Copy link
Contributor Author

I do feel that pnpm config get and pnpm config set are still incomplete. Especially now that pnpm has expand its config structure to more nested objects.

pnpm config get can get an object, but not the property of that object. Perhaps we could make pnpm config get packageExtensions react dependencies to access config.packageExtensions.react.dependencies? Or perhaps we should make pnpm config get ignores pnpm-workspace.yaml entirely?

@zkochan
Copy link
Member

zkochan commented Jul 29, 2025

Yes, probably we should support that. But I think the field names should be separated by dots:

pnpm config get packageExtensions.react.dependencies

@KSXGitHub
Copy link
Contributor Author

Yes, probably we should support that. But I think the field names should be separated by dots

jq does that. And looks good too. The problem is how are we suppose to escape dots in property name? Do we make another property path parser similar to jq? Is there an existing library for that?

@KSXGitHub
Copy link
Contributor Author

And escaping is necessary because dependency names can contain all kinds of symbols: @, /, ., etc.

@zkochan
Copy link
Member

zkochan commented Jul 29, 2025

See how Yarn also supports this: https://yarnpkg.com/cli/config/set

We can check what library Yarn uses. It should be something small.

@KSXGitHub KSXGitHub force-pushed the pnpm-config-get-object-object-9797 branch 4 times, most recently from 3b1a9ef to e28f84f Compare August 3, 2025 20:04
@zkochan
Copy link
Member

zkochan commented Aug 3, 2025

I don't see any usage of the new library in config get/set

@KSXGitHub
Copy link
Contributor Author

I don't see any usage of the new library in config get/set

It's WIP. I haven't glued them together yet. But doing so should be easy.

@KSXGitHub KSXGitHub force-pushed the pnpm-config-get-object-object-9797 branch 2 times, most recently from 8161463 to 0dc5e8d Compare August 4, 2025 15:45
@KSXGitHub KSXGitHub force-pushed the pnpm-config-get-object-object-9797 branch from 0dc5e8d to aeef6df Compare August 5, 2025 21:31
@KSXGitHub
Copy link
Contributor Author

Discovered flaws

  • pnpm config get _authToken would emit an error from npm, but pnpm config list shows it regardless.
  • pnpm config set <npm-setting> would write to ~/.npmrc despite local pnpm-workspace.yaml exist.

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.

pnpm get config on properties with object values returns [object Object]
2 participants