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(deps): remove incorrectly duplicated peerDeps #511

Merged
merged 1 commit into from
Feb 13, 2024

Conversation

agilgur5
Copy link
Contributor

@agilgur5 agilgur5 commented Jan 6, 2024

Motivation

  • these are all inside of prod dependencies already (it has to be either or)

  • notably, antd had a version mismatch between dependencies and peerDependencies (and it was a major version difference, v5 vs v4 after build(deps): bump antd from 4.21.3 to 5.6.1 #387)

    • dependencies supersede peerDependencies however, so the v5 dep was already used and the v4 dep was extraneous
    • see the yarn.lock file as evidence, as well as a downstream yarn.lock, such as Argo Workflows's
      • note how this commit has no changes to the yarn.lock file either
    • this mismatch did cause install warnings downstream as well, which were impossible to resolve given this mismatch in argo-ui's own deps
      • so those should now be resolved as well

Noticed this in argoproj/argo-workflows#12097 (comment)

Modifications

Remove duplicate deps from peerDependencies in package.json

Verification

No changes to yarn.lock, yarn install, build etc all still work fine. CI passes

- these are all inside of prod `dependencies` already (choose one)

- notably, `antd` had a version mismatch between `dependencies` and `peerDependencies` (v5 vs v4)
  - `dependencies` supersede `peerDependencies` however, so the v5 dep was already used and the v4 dep was extraneous
  - see the [`yarn.lock` file](https://github.com/argoproj/argo-ui/blob/c65a9520366b0c0cf0ac585618d029e60041a94d/yarn.lock#L5124) as evidence, as well as a downstream `yarn.lock`, such as [Argo Workflows's](https://github.com/argoproj/argo-workflows/blob/93914261cff4216561c89c1f5f6123e7ad0d5f61/ui/yarn.lock#L2812)
    - note how this commit has no changes to the `yarn.lock` file either
  - this mismatch did cause install warnings downstream as well, which were impossible to resolve given this mismatch in `argo-ui`'s own deps
    - so those should now be resolved as well

Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
Copy link
Member

@crenshaw-dev crenshaw-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works fine w/ Argo CD UI. Thanks!

@crenshaw-dev crenshaw-dev merged commit 7613888 into argoproj:master Feb 13, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
javascript Pull requests that update Javascript code type/dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants