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

chore(deps): remove unneeded Yarn resolutions #11641

Merged
merged 1 commit into from
Aug 25, 2023

Conversation

agilgur5
Copy link
Member

@agilgur5 agilgur5 commented Aug 21, 2023

Motivation

  • removing these has no impact on resolution, as can be seen in the yarn.lock
    • any bumps in these deps will produce either the same minor or same major version (most are same minor version) -- i.e. no breaking changes
    • basically, these are extraneous as of this time

Related to #11630, which removes unused deps; this removes unneeded resolutions

Modifications

Verification

Install succeeds, build passes.
Spot check on render.

@agilgur5 agilgur5 added type/dependencies PRs and issues specific to updating dependencies area/ui javascript Pull requests that update Javascript dependencies labels Aug 21, 2023
- removing these has no impact on resolution, as can be seen in the `yarn.lock`
  - any bumps in these deps will produce either the same minor or same major version (most are same minor version) -- i.e. no breaking changes

- remove resolutions for `@types/react`, `autolinker`, `fast-json-patch`, `lodash`, and `prismjs`

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

@terrytangyuan terrytangyuan left a comment

Choose a reason for hiding this comment

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

I remember these resolutions were added to resolve security issues.

@agilgur5
Copy link
Member Author

agilgur5 commented Aug 25, 2023

Per the first line of the PR, removing these has no impact on resolution. I imagine that those resolutions are either outdated or were used incorrectly to update a version in yarn.lock. Looking at #10842, that was indeed used to update a version in yarn.lock.

yarn.lock can just be updated directly/independently if the bump is within the SemVer range; it does not need a resolution. Can just run yarn install after and it will update your node_modules to match the new lockfile (and make any other auto-updates or throw an error if there's an issue). yarn upgrade can also be used to similar effect, though you often have to specify a version (or range or tag).
You will need to comment out the --frozen-lockfile flag temporarily in the .yarnrc though. Normally that flag is only set in CI and not during development (it was made as a default here, seemingly so that it doesn't have to be individually set in multiple CI builds?)

Copy link
Member

@terrytangyuan terrytangyuan left a comment

Choose a reason for hiding this comment

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

We can merge and see if Snyk check passes

@terrytangyuan terrytangyuan merged commit 39ff284 into argoproj:master Aug 25, 2023
23 checks passed
@agilgur5 agilgur5 deleted the deps-remove-resolutions branch August 25, 2023 20:32
@agilgur5
Copy link
Member Author

It did 😉 .
This is basically a no-op as all the versions in the yarn.lock don't change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ui javascript Pull requests that update Javascript dependencies type/dependencies PRs and issues specific to updating dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants