Skip to content
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

fix: respect scoped registries from publishConfig #844

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Kenneth-Sills
Copy link

@Kenneth-Sills Kenneth-Sills commented Jul 30, 2024

This stops a niche issue when using Gitlab registries, where setting the .npmrc to pull packages from the instance-wide registry while also setting the package.json:publishConfig to push to the project-local registry for a given @scope would pass the incorrect --registry flag to npm publish as well as associate the NPM_TOKEN with the wrong URL in the generated userconfig file.

This DOES NOT resolve #219. So --registry is still technically ignored for scoped packages. See
npm/cli#7043 for more details on that.

I made sure not to affect the priority of other resolutions to avoid any breaking changes. An additional test has been added for this specific case.

EDIT: Fixed description to make it clear I did not fix #219.

This stops a niche issue when using Gitlab registries, where setting the
`.npmrc` to pull packages from the instance-wide registry while also setting
the `package.json:publishConfig` to push to the project-local registry
for a given `@scope` would pass the incorrect `--registry` flag to `npm publish`
as well as associate the `NPM_TOKEN` with the wrong URL in the
generated userconfig file.

This DOES NOT resolve semantic-release#219. So `--registry` is still technically ignored for
scoped packages. See
[npm/cli#7043](npm/cli#7043)
for more details on that.

I made sure not to affect the priority of other resolutions to avoid any
breaking changes. An additional test has been added for this specific
case.
@travi
Copy link
Member

travi commented Jul 30, 2024

Could you please provide a public reproduction of the problem? What you describe is the default npm behavior, which we do or best to align with already by mostly deferring to npm to handle. I've used such configuration myself without complication, so I'm skeptical of this being a problem

@Kenneth-Sills
Copy link
Author

Kenneth-Sills commented Jul 30, 2024

Thanks for your time, @travi!

I don't have a way to reproduce publicly right now, as this is happening in a private self-hosted instance of Gitlab. But I can go into a little more detail.

To define NPM's behavior, assuming a package called @demo/package...

Scopeless

// package.json
{
    "publishConfig": {
        "registry": "https://pkg.fake.io"
    }
}
# .npmrc
registry=https://rc.fake.io
# Command
npm publish $(pwd) --registry "https://cli.fake.io"

# Attempts to publish to 
# https://cli.fake.io

Scoped in the .npmrc

// package.json
{
    "publishConfig": {
        "registry": "https://pkg.fake.io"
    }
}
# .npmrc
@demo:registry=https://rc.fake.io
# Command
npm publish $(pwd) --registry "https://cli.fake.io"

# Attempts to publish to 
# https://rc.fake.io

Scoped in Both .npmrc and publishConfig

// package.json
{
    "publishConfig": {
        "@demo:registry": "https://pkg.fake.io"
    }
}
# .npmrc
@demo:registry=https://rc.fake.io
# Command
npm publish $(pwd) --registry "https://cli.fake.io"

# Attempts to publish to 
# https://pkg.fake.io

This plugin currently only discovers the register field of publishConfig, but it will never recognize a @scope:registry field. Meanwhile, if you look at the changeset, you'll see it does locate scoped registry entries in the project's .npmrc.

So there's no issue in the first two cases above, but on the last case - "Scoped in Both .npmrc and publishConfig" - this plugin deviates from NPM's behavior. When it sets up the userconfig, it will call getRegistry, which will fail to find @demo:registry in the package.json:publishConfig. Instead, it will return the scoped registry from .npmrc and then set up the environment's NPM_TOKEN for that environment instead of the registry npm publish will actually use and authorization will fail when attempting to push.

This change just adds the ability to locate those scoped registries in publishConfig and have them override the entries in .npmrc.


EDIT: In the interim, I've added a separate step to my Gitlab CI/CD to run npm config set -L project just before any semantic release commands (having dependency installation handled by a job in an earlier stage).

@Kenneth-Sills
Copy link
Author

Another way to confirm this change is needed is just to reset the changes to lib/get-registry.js and the run the new test:

[test:unit       ]   get-registry › Get the registry configured in "publishConfig" for scoped package
[test:unit       ] 
[test:unit       ]   test/get-registry.test.js:47
[test:unit       ] 
[test:unit       ]    46:                 
[test:unit       ]    47:   t.is(         
[test:unit       ]    48:     getRegistry(
[test:unit       ] 
[test:unit       ]   Difference (- actual, + expected):
[test:unit       ] 
[test:unit       ]   - 'https://custom6.registry.com'
[test:unit       ]   + 'https://custom5.registry.com'
[test:unit       ] 
[test:unit       ]   › file://test/get-registry.test.js:47:5

It's unable to see the scoped registry under publishConfig.

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.

Use CLI --@scope:registry flag to override scoped registry defined in .npmrc
2 participants