Skip to content

fix(publish.yml): add registry-url to old publish workflow#1009

Merged
DreaminDani merged 1 commit intomainfrom
dani-npm-registry-fix
Apr 21, 2026
Merged

fix(publish.yml): add registry-url to old publish workflow#1009
DreaminDani merged 1 commit intomainfrom
dani-npm-registry-fix

Conversation

@DreaminDani
Copy link
Copy Markdown
Contributor

Trying to revive the old publish workflow

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Apr 21, 2026

⚠️ No Changeset found

Latest commit: b52ef94

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 21, 2026

Claude finished @DreaminDani's task in 1m 23s —— View job


PR Review: fix(publish.yml): add registry-url to old publish workflow

  • Gather context
  • Read changed files
  • Provide review feedback

Good fix! The registry-url addition is the right call — actions/setup-node only writes NODE_AUTH_TOKEN into .npmrc when registry-url is set, so without it OIDC-backed npm publish silently fails with ENEEDAUTH.

One thing worth flagging for clarity:

💡 Suggestion (non-blocking)

This PR's diff against main includes 6 lines total, but the title only mentions registry-url. It also re-adds the "Upgrade npm for OIDC support" step (npm install -g npm@latest) that was explicitly removed in #1007 ("chore(publish.yml): remove upgrading npm step"). If that re-addition is intentional, it's fine — but it's worth calling out since main already has the comment "Node 22 includes npm >= 11.5.1 with OIDC support" suggesting the upgrade step shouldn't be necessary. If Node 22's bundled npm is sufficient, the upgrade step adds ~30s of install time and a floating-version footgun (@latest). Either way, a sentence in the PR description clarifying the intent would help future readers.

Technically the approach is sound — registry-url is set before the npm upgrade so .npmrc is already in place, and NODE_AUTH_TOKEN is env-based so the upgraded npm will still pick it up. No correctness issues.

registry-url fix is correct and should resolve the auth problem.

@workflow-authentication-public
Copy link
Copy Markdown
Contributor

📚 Storybook Preview Deployed

✅ Preview URL: https://click-48wxxw23b-clickhouse.vercel.app

Built from commit: 38a9801aedf05e6e93d6569bc8afcea003fbebc2

@DreaminDani DreaminDani merged commit 4f79e49 into main Apr 21, 2026
11 checks passed
@DreaminDani DreaminDani deleted the dani-npm-registry-fix branch April 21, 2026 20:32
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.

2 participants