Skip to content

feat: support read scoped registry from package json#1470

Merged
Andarist merged 7 commits intochangesets:mainfrom
JounQin:feat/scope-registry
Apr 12, 2025
Merged

feat: support read scoped registry from package json#1470
Andarist merged 7 commits intochangesets:mainfrom
JounQin:feat/scope-registry

Conversation

@JounQin
Copy link
Copy Markdown
Contributor

@JounQin JounQin commented Sep 25, 2024

close #1469

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Sep 25, 2024

🦋 Changeset detected

Latest commit: fdf8084

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@changesets/cli Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@codecov
Copy link
Copy Markdown

codecov bot commented Sep 25, 2024

Codecov Report

Attention: Patch coverage is 0% with 22 lines in your changes missing coverage. Please review.

Project coverage is 80.66%. Comparing base (dc83cb4) to head (fdf8084).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
packages/cli/src/commands/publish/npm-utils.ts 0.00% 21 Missing ⚠️
...ckages/cli/src/commands/publish/publishPackages.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1470      +/-   ##
==========================================
- Coverage   81.16%   80.66%   -0.51%     
==========================================
  Files          54       54              
  Lines        2241     2255      +14     
  Branches      674      681       +7     
==========================================
  Hits         1819     1819              
- Misses        417      431      +14     
  Partials        5        5              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JounQin
Copy link
Copy Markdown
Contributor Author

JounQin commented Sep 25, 2024

Not sure how to add related test cases.

cc @Andarist

Copy link
Copy Markdown
Contributor

@bluwy bluwy left a comment

Choose a reason for hiding this comment

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

Seems good to me. Docs for publishConfig['@scope:registry'] is limited, but it looks like it works according to #1272 (and the references within it, which also resulted in this PR?)

@JounQin
Copy link
Copy Markdown
Contributor Author

JounQin commented Oct 11, 2024

@Andarist Any time to review this PR?

@RodrigoHamuy
Copy link
Copy Markdown
Contributor

Alo, just checking when can we merge this as this is an issue for us since we are using yarn and private Github Packages.

@JounQin
Copy link
Copy Markdown
Contributor Author

JounQin commented Apr 7, 2025

friendly ping @Andarist

Copy link
Copy Markdown
Member

@Andarist Andarist left a comment

Choose a reason for hiding this comment

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

Overall looks good but I left some comments that have to be addressed before we land this.

@JounQin JounQin requested a review from Andarist April 8, 2025 13:47
@JounQin
Copy link
Copy Markdown
Contributor Author

JounQin commented Apr 11, 2025

@Andarist Any further review?

@Andarist
Copy link
Copy Markdown
Member

@JounQin could you double-check the changes I made in b932a83 ?

? "https://registry.npmjs.org"
: registry;
return {
scope: undefined,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
scope: undefined,

Given it's optional, we can omit this.

@JounQin
Copy link
Copy Markdown
Contributor Author

JounQin commented Apr 11, 2025

@JounQin could you double-check the changes I made in b932a83 ?

@Andarist Good cleaner refactor, LGTM, just left a unimportant comment/suggestion.

False positive, so make it happy
"@changesets/cli": minor
---

Support scoped registries configured using `package.json#publishConfig` and environment variables
Copy link
Copy Markdown
Contributor Author

@JounQin JounQin Apr 11, 2025

Choose a reason for hiding this comment

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

I think we should mention more specifically what's the format in case that's really not well known, maybe linking to the @npm/cli or #1469 would be better.

const scope = packageName.split("/")[0];
const scopedRegistry =
packageJson!.publishConfig?.[`${scope}:registry`] ||
process.env[`npm_config_${scope}:registry`];
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

last check before merging this - have you tried using this env variable? I know for sure it works with packageJson.publichConfig but I have not tested this with the env variable path

Copy link
Copy Markdown
Contributor Author

@JounQin JounQin Apr 12, 2025

Choose a reason for hiding this comment

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

@Andarist It's impossible to set the env directly via cli or envrc or export, because the environment name contains @ and :, but it's possible to set it with exec#env override.

image

@Andarist Andarist merged commit 29f34a3 into changesets:main Apr 12, 2025
3 checks passed
@github-actions github-actions bot mentioned this pull request Apr 12, 2025
@JounQin JounQin deleted the feat/scope-registry branch April 12, 2025 09:44
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.

[cli] npm info should respect publishConfig#@private:registry config

5 participants