-
Notifications
You must be signed in to change notification settings - Fork 0
[Feature] Improve yarn npm publish to match other package managers #2
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
Conversation
…lish Co-authored-by: Saadnajmi <[email protected]>
Co-authored-by: Saadnajmi <[email protected]>
|
@copilot the executeWithJsonOutput code path seems to have a lot of duplicate code with the stream path. Can we consolidate better? |
…m publish Co-authored-by: Saadnajmi <[email protected]>
I've consolidated the duplicate code by extracting common logic into reusable private methods. The refactoring eliminates ~60 lines of duplication while maintaining identical functionality. Both JSON and stream output paths now use the same core logic with different reporting mechanisms. See commit 28df538. |
Co-authored-by: Saadnajmi <[email protected]>
Co-authored-by: Saadnajmi <[email protected]>
|
@copilot the class StreamReport at |
…n paths Co-authored-by: Saadnajmi <[email protected]>
You're absolutely right! I've refactored the code to use StreamReport's built-in JSON support instead of maintaining separate execution paths. This simplifies the implementation by:
See commit 596cf55. |
| otp: this.otp, | ||
| jsonResponse: true, | ||
| }); | ||
| private async executeCore(workspace: any, registry: string, configuration: any, ident: any, version: string, report: any) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot now that we have one execute path, we don't need executeCore anymore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the unnecessary executeCore method and consolidated all logic into a single execution path. The duplicate method has been eliminated while maintaining identical functionality. See commit 22b0854.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot we also factored out methods like checkTolerateRepublish, determineProminence, reportProminenceDesision, and performPackAndPublish. That resulted in any types. Now that we have one execute flow again, can we just fold those back into the one method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Folded all extracted methods back into the single execute method to eliminate the any types. The code now has proper TypeScript typing since all variables are declared in the correct scope. See commit ee34047.
Co-authored-by: Saadnajmi <[email protected]>
…tion path Co-authored-by: Saadnajmi <[email protected]>
…ny types Co-authored-by: Saadnajmi <[email protected]>
… automatically Co-authored-by: Saadnajmi <[email protected]>
#6850) ## What's the problem this PR addresses? Addresses #6849 ## How did you fix it? Full disclosure, used Github Copilot Agent Mode to generate most of this PR here --> Saadnajmi#2 Since then, lots more manual edits on my side. I took some spirit from #3404 and `pack.ts` to determine how to print paths and when to call `report.reportInfo` vs `report.reportJson`. This adds a couple of flags to `yarn npm publish`, so that it matches other package managers like `pnpm` and `bun`, and is thus easier for tools like `nx release` to use (see nrwl/nx#29242 ). We add - `--dry-run` (Bails out right before the final publish to NPM) - `--json` (This takes advantage of StreamReports JSON config) Using both flags together looks something like this: ```shell sanajmi@Mac foo % yarn npm publish --dry-run ➤ YN0000: README.md ➤ YN0000: bar.js ➤ YN0000: package.json ➤ YN0000: xyz.js ➤ YN0000: [DRY RUN] Package would be published to https://registry.yarnpkg.com with tag latest ➤ YN0000: Done in 0s 13ms sanajmi@Mac foo % yarn npm publish --dry-run --json {"file":"README.md"} {"file":"bar.js"} {"file":"package.json"} {"file":"xyz.js"} {"name":"foo","version":"1.0.0","registry":"https://registry.yarnpkg.com","tag":"latest","files":["README.md","bar.js","package.json","xyz.js"],"access":null,"dryRun":true,"published":false,"message":"[DRY RUN] Package would be published to https://registry.yarnpkg.com with tag latest","provenance":false,"gitHead":"d4f82af6fdb7f2f81c5910535900c5ad8a70a019"} ``` ## Checklist - [x] I have read the [Contributing Guide](https://yarnpkg.com/advanced/contributing). (TODO) <!-- See https://yarnpkg.com/advanced/contributing#preparing-your-pr-to-be-released for more details. --> <!-- Check with `yarn version check` and fix with `yarn version check -i` --> - [ ] I have set the packages that need to be released for my changes to be effective. <!-- The "Testing chores" workflow validates that your PR follows our guidelines. --> <!-- If it doesn't pass, click on it to see details as to what your PR might be missing. --> - [ ] I will check that all automated PR checks pass before the PR gets reviewed. --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: Saadnajmi <[email protected]>
This PR implements new features for the
yarn npm publishcommand to achieve feature parity with other package managers likepnpmandbun, as requested in the issue.New Features Added
1.
--dry-runflagShows what would be published without actually publishing the package:
2.
--jsonflagOutputs results in JSON format instead of human-readable format:
3.
--registryflagAllows overriding the registry configuration:
4. Directory positional argument
Allows specifying a working directory to publish from:
Implementation Details
clipanionlibraryppath.resolve()Testing
All features have been manually tested and work correctly:
--dry-runprevents actual publishing while showing what would be published--jsonoutputs proper JSON structure with all relevant metadata--registrysuccessfully overrides default registry configurationThe implementation follows the existing codebase patterns and maintains full backward compatibility.
Fixes #1.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.