package.json overrides option

[coreyfarrell]

Cross-posted to https://npm.community/t/package-json-overrides-option/9265

[isaacs]

I like this for overriding these fields with unreserved +1:

• dependencies
• devDependencies
• optionalDependencies
• peerDependencies
• main
• directories

I worry about updating the bin section since it could mean that the presented executable interface is not consistent. Maybe it should stipulate that the bin values can be different, but the keys must be identical in all environments? Same for scripts: definite value in switching, but they should be required to have the same keys.

We should not allow overriding files, since that would only be relevant when creating a package, and you definitely want all the files in the tarball. (In fact, one caveat here is that npm-packlist must include the main/bin/directories entries for all environments in order to be overrides-compatible.)

config seems like a strange one. I'm having trouble imagining a scenario where that'd be useful.

I am wondering if we should allow overriding unspecified user fields as well. For example, it might be nice to be able to pass different configs to nyc or tap or babel based on node version. Of course, that means those tools would have to be aware of how overrides work, or use read-package-json instead of just loading the json and parsing it, which is pretty heavyweight.

If we don't allow overriding user fields, then I think it's a good idea to explicitly limit exactly which fields are allowed in the overrides section, and raise an error if anything else is in there, to steer people away from footguns.

• Should filtering fields and manipulation fields be placed in a separate sub-objects? I feel like this would be too verbose.

I agree. Since you can't override any of the fields used for filtering, it's unambiguous.

• Should npm publish completely ignore all overrides? Maybe config should not be allowed in overrides?

Yeah, I think we leave config and publishConfig out of it.

So, the places where this needs to be implemented:

• read-package-json
• npm-packlist

relation to other RFCs

I was thinking that we'd need to call this something other than "overrides", since there is already an accepted RFC with that name, but as I think on this more, I kind of like this approach better than that one. I think a pretty good plan is maybe to ratify this, de-ratify the current overrides, and instead add a new RFC to implement yarn's resolutions with the same syntax and logic as yarn uses. Better to walk the existing cowpath than create another one.

This is similar to #28, but much lighterweight and easier to implement. It is less powerful in the expression and filtering syntax, but I think that's a feature rather than a bug.

[coreyfarrell]

Maybe it should stipulate that the bin values can be different, but the keys must be identical in all environments?

For bin maybe the restriction is that override sections cannot define any new scripts nor set null. So you can only override, but you don't have to override all of them. Say you have "bin": {"maincmd": "./generic/maincmd.js", "othercmd": "./generic/othercmd.js"}. Maybe win32 needs completely different initialization for maincmd only, you should be able to put "bin": {"maincmd": "./win32/maincmd.js"} in the override section and it should retain the othercmd. So package.bin = {...package.bin, ...package.overrides[idx].bin}. One potential use I can see for bin overrides is a package could be written with modern JS targeting v8-7.5 and use babel to transpile for node.js 6 compatibility. Source and transpiled result are both published to files, package.json linked to the transpiled results by default but provides overrides for the appropriate version to use the original sources.

Same for scripts: definite value in switching, but they should be required to have the same keys.

One concern with allowing override of scripts is what should happen to the scripts that run as part of npm publish or npm pack? Granted the scripts can currently call something which takes conditional action. I'm not really sure it's fully needed.

Although the same argument could be made about bin files this makes it easy to have npm install link to a script most optimized for current the platform for example maybe by default link to bins that were transpiled with @babel/preset-env but provide allow the original bin to be used for node.js 12.

We should not allow overriding files, since that would only be relevant when creating a package, and you definitely want all the files in the tarball. (In fact, one caveat here is that npm-packlist must include the main/bin/directories entries for all environments in order to be overrides-compatible.)

Yes this would need to be clearly documented. Also I'm not sure if npm pack / npm publish will throw an error if bin entries point to nothing but it should.

I am wondering if we should allow overriding unspecified user fields as well. For example, it might be nice to be able to pass different configs to nyc or tap or babel based on node version. Of course, that means those tools would have to be aware of how overrides work, or use read-package-json instead of just loading the json and parsing it, which is pretty heavyweight.

If we don't allow overriding user fields, then I think it's a good idea to explicitly limit exactly which fields are allowed in the overrides section, and raise an error if anything else is in there, to steer people away from footguns.

I think npm should be agnostic on this for forward compatibility. If we raise an error on unknown/user fields it would be impossible to introduce new override-able fields in the future. A way to print warnings about unknown and user fields would be nice but I don't think an error would be good. Also I don't think it would be good to print those warnings by default. Especially npm install someone-elses-package should not tell me about known overrides in that package, these would be maintainer warnings.

Yeah, I think we leave config and publishConfig out of it.

Probably a good idea at least to start. Maybe once overrides are a thing someone will give a good reason why it's needed.

[isaacs]

This should stipulate that overrides are a deep merge. Ie, if you have:

{
  "dependencies": {
    "a": "1",
    "b": "2"
  },
  "overrides": [
    {
      "engines": { "node": "<= 6" },
      "dependencies": { "b": "1" }
    }
  ]
}

then on node v6, the dependencies will be {"a":"1","b":"1"}.

Discussed in slack: how does this affect package-lock? Seems tricky, since it can lead to a combinatorial explosion if override filters aren't identical. Since engines.node can be a semver range, we get into some really hairy situations if you have multiple different modules in the tree with different and overlapping range selectors.

[coreyfarrell]

Discussed in slack: how does this affect package-lock? Seems tricky, since it can lead to a combinatorial explosion if override filters aren't identical. Since engines.node can be a semver range, we get into some really hairy situations if you have multiple different modules in the tree with different and overlapping range selectors.

A standard way to configure CI could be a solution to this. This would allow the CI environments and npm to have the same information. npm could use this information to generate a distinct list of overrides in package-lock.json and CI could use it to decide what versions / platforms to test with. This would require that npm have a way to determine the latest released versions of node (I assume accomplishing this would be minor). Maybe the default behavior should be to simply produce overrides covering the latest version of each semver-major. In theory it could detect that no special overrides cover node.js 9 so the overrides for v8 might cover >=8.3 <10.13, assuming that node.js ^10.13 is the range which covers v10.

My concern with only allowing semver-major for the overrides is that some authors will be pedantic. They might want to use >=8.3 for packages which use object rest/spread and if only semver-major could be specified they might say that >=9 is required.

[coreyfarrell]

Abandoned in favor of #72.