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

addendum: overrides apply if value matches, as well as key #441

Merged
merged 1 commit into from
Sep 2, 2021

Conversation

isaacs
Copy link
Contributor

@isaacs isaacs commented Aug 26, 2021

Found a pretty serious problem with overrides while starting
implementation. Namely, it is in many cases impossible to determine
whether a node in a reified tree was subject to an override or not.

The edge case is described in this patch, and a fix proposed:

If a dependency matches the key in an overrides object or it
matches the "." value specifier, then the override ruleset will
apply.

By doing this, it prevents version-swapping, makes infinite regress even
more impossible, and ensures that the resulting package tree can always
be properly examined, without any out of band bookkeeping.

cc: @nlf

References

@isaacs isaacs requested a review from nlf August 26, 2021 19:28
@isaacs isaacs added the Agenda will be discussed at the Open RFC call label Aug 26, 2021
@ljharb
Copy link
Contributor

ljharb commented Aug 26, 2021

Is there any negative consequence of this change - like, any lost capability or new caveats to be aware of as an end user?

@isaacs
Copy link
Contributor Author

isaacs commented Aug 26, 2021

Is there any negative consequence of this change - like, any lost capability or new caveats to be aware of as an end user?

Just potential confusion.

For example, say you have a package.json like this:

{
  "dependencies": {
    "foo": "1 || 2"
  },
  "overrides": {
    "foo@1": {
      ".": "2",
      "bar": "1.2.3"
    }
  }
}

If you install it, and foo@1 || 2 from the dependencies results in foo@2.3.4, then it will still get bar@1.2.3 as an overridden dependency, because the . member matches foo@2.3.4, even though the foo@1 key clearly doesn't.

Far and away, the most common use case will be to lock down all versions of a given library within a project, like:

{
  "dependencies": {
    "foo": "1 || 2"
  },
  "overrides": {
    "react": "16",
    "react-dom": "16"
  }
}

so I do expect that this will be an edge case.

Found a pretty serious problem with overrides while starting
implementation.  Namely, it is in many cases impossible to determine
whether a node in a reified tree was subject to an override or not.

The edge case is described in this patch, and a fix proposed:

> **If a dependency matches the `key` in an overrides object _or_ it
> matches the `"."` value specifier, then the override ruleset will
> apply.**

By doing this, it prevents version-swapping, makes infinite regress even
more impossible, and ensures that the resulting package tree can always
be properly examined, without any out of band bookkeeping.

cc: @nlf

PR-URL: #441
Credit: @isaacs
Close: #441
Reviewed-by: @isaacs
@isaacs isaacs force-pushed the isaacs/overrides-addendum branch from c94c7f4 to 9564c0d Compare September 2, 2021 22:54
@isaacs isaacs closed this in 9564c0d Sep 2, 2021
@isaacs isaacs merged commit 9564c0d into latest Sep 2, 2021
@isaacs isaacs deleted the isaacs/overrides-addendum branch September 2, 2021 22:54
@darcyclarke darcyclarke removed the Agenda will be discussed at the Open RFC call label Sep 15, 2021
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.

3 participants