Skip to content

ci(deps): dedupe yarn.lock, add check for dupes#11637

Merged
terrytangyuan merged 2 commits intoargoproj:masterfrom
agilgur5:ci-yarn-dedupe
Aug 28, 2023
Merged

ci(deps): dedupe yarn.lock, add check for dupes#11637
terrytangyuan merged 2 commits intoargoproj:masterfrom
agilgur5:ci-yarn-dedupe

Conversation

@agilgur5
Copy link
Copy Markdown

@agilgur5 agilgur5 commented Aug 20, 2023

Motivation

Use as few duplicates of dependencies as possible, reducing install size, memory usage, and bundle size

Modifications

  • add a deduplicate script that uses yarn-deduplicate
  • run deduplicate during make lint and UI CI
    • I added this as another one-liner in make lint b/c that was honestly much more readable than trying to do multi-line shell conditionals inside of a Makefile (which require \ and ; as they get interpreted as a single line)
  • run deduplicate once and commit the results
    • double-checked 50%+ of these automated changes and they all look good to me

similar to a commit I made in a library I maintained a few years ago: jaredpalmer/tsdx@22133ce (part of jaredpalmer/tsdx#683)

Also, upgrade ts-loader to v8.4.0

Verification

Ran the new make lint: confirmed it worked and did not make an additional diff.
Spot check on UI render was ok.
CI checks pass

Notes to Reviewers

This will likely merge conflict hard with a few my other open UI deps PRs, so it probably makes more sense to merge those first and then update this PR after those are all merged

@agilgur5
Copy link
Copy Markdown
Author

Huh Prod UI build is failing on a ts-loader error. Was able to reproduce that locally, but it doesn't seem to occur in dev mode probably because type-checking is off.

That error suggests that one of the deps was relying on an outdated version of TS

@agilgur5
Copy link
Copy Markdown
Author

Ah yep, TS 4.6 was deduped and ts-loader needs an update to support TS 4.7+.
(For reference, TS does not follow SemVer, so there can be breaking changes in minors like this one)

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.

Can we combine this with other similar PRs?

@agilgur5
Copy link
Copy Markdown
Author

So all 3 (#11641, #11630, and this one) are technically independent of each other.
In particular, this is the only one that massively merge conflicts when there's a dep change, the other two can be merged pretty cleanly. Pretty sure both of those can be merged right now.

@terrytangyuan terrytangyuan enabled auto-merge (squash) August 25, 2023 19:48
@agilgur5
Copy link
Copy Markdown
Author

agilgur5 commented Aug 25, 2023

One E2E test flake (I believe I've seen this one before too from a quick glance).
And unit tests failed on something completely unrelated in Go code -- that looks like potentially a race condition or something in the test? (it tried running both the dummy metrics server and the real one simultaneously)

Anton Gilgur added 2 commits August 28, 2023 16:37
- add a `deduplicate` script that uses `yarn-deduplicate`
- run `deduplicate` during `make lint` and UI CI
- run `deduplicate` once and commit the results
  - double-checked 50%+ of these automated changes and they all look good to me

- similar to a commit I made in a library I maintained a few years ago: jaredpalmer/tsdx@22133ce

Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
- due to a breaking change in TS 4.7 (TS does not follow SemVer), `ts-loader` needed an update: https://github.com/TypeStrong/ts-loader/blob/v9.4.4/CHANGELOG.md#v840

- `ts-loader` [v8](https://github.com/TypeStrong/ts-loader/blob/v9.4.4/CHANGELOG.md#v800)'s only breaking change was to no longer support TS <3.6

Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
auto-merge was automatically disabled August 28, 2023 20:38

Head branch was pushed to by a user without write access

@agilgur5
Copy link
Copy Markdown
Author

Geh, some other PR ended up merge conflicting as well 😭 Possibly #11678.

Rebased on top

@terrytangyuan terrytangyuan merged commit 74551e3 into argoproj:master Aug 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/build Build or GithubAction/CI issues 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