Skip to content

fix(publish): --config.unknown CLI options needs special handling when passing them to npm #9085

Closed
naruaway wants to merge 1 commit intopnpm:mainfrom
naruaway:fix-publish-cli-unknown-option-handling
Closed

fix(publish): --config.unknown CLI options needs special handling when passing them to npm #9085
naruaway wants to merge 1 commit intopnpm:mainfrom
naruaway:fix-publish-cli-unknown-option-handling

Conversation

@naruaway
Copy link
Contributor

@naruaway naruaway commented Feb 12, 2025

This should fix #9084

If this direction looks good, I can add tests

So far I quickly verified by modifying dist/pnpm.cjs directly inside node_modules for the corresponding place

@welcome
Copy link

welcome bot commented Feb 12, 2025

💖 Thanks for opening this pull request! 💖
Please be patient and we will get back to you as soon as we can.

@naruaway naruaway changed the title fix(publish): it shoud handle --config.unknown CLI options fix(publish): --config.unknown CLI options needs special handling when passing them to npm Feb 12, 2025
@naruaway naruaway force-pushed the fix-publish-cli-unknown-option-handling branch from 873e398 to 4867e32 Compare February 12, 2025 00:27
@zkochan
Copy link
Member

zkochan commented Feb 12, 2025

Yes, this is probably OK.

}
}
// Convert Pnpm's "unknown config" form into regular form so that npm can understand it
args = args.map(a => a.replace(/^--config\./, '--'))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: I just realized that we need to make sure we should not apply this conversion after --. I'll fix later after getting some signal from maintainers

@naruaway
Copy link
Contributor Author

@zkochan Thanks for really quick response! I'll make sure handle edge cases and will add tests later today

@naruaway
Copy link
Contributor Author

After some consideration, now I would prefer #9089 over this PR. Closing for now

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.

Cannot override scoped registry for "pnpm publish" via CLI options

2 participants