--registry argument should override package.json publishConfig.registry #5522

Open
drkibitz opened this Issue Jun 21, 2014 · 16 comments

Comments

Projects
None yet
10 participants

I understand there is currently a lot of development happening, big changes, feature implementations, etc., but I thought this may be a small request.

Currently the package.json publishConfig.registry field overrides everything, including the value passed with --registry argument. This is troublesome because multiple registries are not supported at this time, and defining publishConfig.registry means it is locked unless a change is made to the package.json to simply publish to a different registry.

As a stopgap solution until multiple registry support is available, I propose simply overriding publishConfig.registry with the one passed via the argument.

Am I missing something that would allow this now? Is is a bug? Is it a feature? Is this doable? Is this a breaking change? I'm not sure.

If a pull request is welcome I would be very interested in this functionality soon and will contribute.

drkibitz changed the title from --registry vs publishConfig.registry to package.json publishConfig.registry overrides --registry argument Jun 21, 2014

Contributor

othiym23 commented Jun 21, 2014

This may seem like a simple-minded question, but why not just delete publishConfig.registry if you want more control over which registry a package is published to?

publishConfig is there as both a convenience and a safety measure -- presumably, if you've added a publishConfig stanza to your package manifest, you want a package to only be published to the specified registry, and making that overridable is a footgun.

@othiym23 You just answered your own question.

why not just delete publishConfig.registry
publishConfig is there as both a convenience and a safety measure

If not using publishConfig, you must assume that everyone has their local environments set up correctly. This is something that is not trusted where I am, deleting it is not an option.

But my wish is to be able to push these packages to different environments before the final environment. This is for various reasons that I don't think I should need to go it into.

How in the heck is having a command-line argument override configuration a footgun? I completely disagree there. This is one of the most common relationships between nix cli tool arguments and configuration, including in the npm client.

There is nothing publishConfig.registry is providing by way of security. So then it's only useful as a per package default which overrides locale configuration. Yes this does assume the package would like to be published to that registry, which would be it's regular final destination, but it may not be a direct flight.

I second this.

When developing internal libraries, publishConfig is the first place to put URL of internal registry - the most obvious and environment-independent place to say "this lib is never intended to be published to public registry". But if we have >1 registry (e.g. dev and production), then we have to override it - and that's impossible. Well, not impossible, but we have to do it manually or write scripts, which feels hacky. I mean, writing a script to configure my package manager?

Using .npmrc for this is not convenient, first of all because it doesn't differentiate between registry to publish and registry to download - and in internal environments URLs are often different (e.g. if using Sonatype Nexus this is the proposed and most often setup).

👍

I'm writing a script to migrate packages from Nodejitsu to (self-hosted) npm Enterprise. I'd like to npm publish <tarball> for each published version of each package, so the full package histories remain accessible when Nodejitsu shuts down. If I'm forced to modify the packages in any way I'll lose the guarantees provided by the checksums.

jkrems commented Jun 30, 2015

Running into the exact same thing as @davidchambers. We have a bunch of packages where versions match git tags in the repositories. And it would be very unfortunate if we'd have to patch files inside of the packages, making the git tags "worthless" just to make publishing work.

jkrems commented Jun 30, 2015

For the record I patched publish.js as a temporary workaround:

  var config = mappedConfig.config
  var registry = mappedConfig.client

  // <HACK>
  if (npm.config.get('force-registry')) {
    log.verbose('Force registry to --registry option, ignoring publishConfig.registry')
    registry = npm.registry
    config = npm.config
  }
  // </HACK>

  data._npmVersion  = npm.version
  data._nodeVersion = process.versions.node

We would love to have this change incorporated. We also don't trust users (especially new developers) to always have their global npm config registry set to our internal npm repository, we'd like to prevent accidentally publishing internal artifacts to the default external global npmjs repository. We use nexus for hosting npm artifacts, and the publish URL (hosted repository) is different from the download URL (group repository consisting of hosted repository plus proxy repository for the external npmjs). Command line values should always override configuration settings.

This was filed as a bug, but the feature-request label makes the title sound like I want the current behavior. Changing title.

drkibitz changed the title from package.json publishConfig.registry overrides --registry argument to --registry argument should override package.json publishConfig.registry Sep 24, 2015

But it still has feature-request label. I would consider it a bug.

gchorny commented Nov 27, 2015

Similarly to @davidchambers , I'm moving some packages with all the dependencies to a development network. But when I try to publish 'builtins', it complains about lack of rights to publish the package, which is apparently because it is trying to publish to outside registry. It's great that the system caught it but bad that I cannot override it.

Contributor

othiym23 commented Jul 20, 2016

It's not a bug, because the current behavior is more or less underspecified and the current behavior is at least reasonable, if not what many users expect. Changing it at this point is actually a semver-major change, because it changes how the CLI handles this piece of configuration.

The correct solution is to change the code in lib/publish.js to get the publishConfig stanza and then check the cli element of config-chain (a tool used internally to the CLI's configuration framework) for a set registry. This is necessary so that the publishConfig can override the registry set in .npmrc, but can be overridden by --registry passed via the command-line (yes, this code is a little excessively baroque).

Since the CLI team is currently focused on fixing big bugs (crashers and things that produce incorrect installs), it may be a while until we get to this. The above ought to be enough to help someone put together a patch, though, and if one comes, we'll work to get it merged speedily. Thanks to all for their patience!

drkibitz commented Jul 20, 2016 edited

@othiym23 I'm not exactly sure, but I think I might have created this issue before package specific .npmrc files were supported. I haven't been doing much npm package administration as of late, but that may or may not cover the use case I was trying to solve for myself here. It would still require overwriting the .npmrc file at build time, but that's better than the actual package.json file itself. I'll have to let others involved in this issue decide if their use cases are covered as well. Thanks!

drkibitz commented Jul 20, 2016 edited

@othiym23 oh I missed what you said about .npmrc being being overridden by --registry. Then that definitely covers the use case I was going for here. That would essentially mean, remove publishConfig from package.json, and use .npmrc instead, which actually behaves as expected regarding this issue.

mhofman commented Sep 7, 2016

For my use case, --registry overriding .npmrc is not sufficient: we use artifactory and a virtual registry that combine both a proxy/cache to remote npmjs and a local repository for internal scoped packages.

For that reason we configure our projects like this:

  • Project's .npmrc has a @scope:registry:https://artifactory/api/npm/public-npm/ to automatically fetch scoped packages from the artifactory's virtual repo so that all developers can install regardless of their local configuration
  • Project's package.json has a publishConfig["@scope:registry"] == "https://artifactory/api/npm/local-npm/" so that the package doesn't get inadvertently published to a wrong registry.
  • Our build machines have the credentials configured in the global .npmrc for the local-npm registry so that only the developers machine don't mistakenly publish a build from their own machine.

This works fine, except when we need to publish packages to another registry, e.g. for pre-release or testing. There is no way to override the publishConfig entry in this case, and we cannot switch to using the project's .npmrc as suggested above since that points to another registry.

givankin referenced this issue in yarnpkg/yarn Oct 11, 2016

Open

Handling of multiple npm servers #547

I second this. Command line arguments / input should always override pre-defined stuff. That's how it works in almost every other CLI thing ever.

FYI for @mhofman, @RobzInadE, and anyone finding and commenting on this issue...
Although I am not in the position to make a patch for this any time soon, as I'm not using npm in my day to day as I once was. Please see that the NPM team has marked this issue with "patch-welcome", so anyone in the community who can, should probably feel free to make a pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment