Skip to content

chore(deps): remove unneeded Yarn resolutions#11641

Merged
terrytangyuan merged 1 commit intoargoproj:masterfrom
agilgur5:deps-remove-resolutions
Aug 25, 2023
Merged

chore(deps): remove unneeded Yarn resolutions#11641
terrytangyuan merged 1 commit intoargoproj:masterfrom
agilgur5:deps-remove-resolutions

Conversation

@agilgur5
Copy link
Copy Markdown

@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.

- 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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
@agilgur5
Copy link
Copy Markdown
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.

2 participants