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: upgrade argo-ui components to latest #11585

Merged
merged 6 commits into from
Aug 16, 2023

Conversation

agilgur5
Copy link
Contributor

@agilgur5 agilgur5 commented Aug 15, 2023

Replaces #11510, fixes all issues I commented about
Fixes #11448 by incorporating argoproj/argo-ui#418

Motivation

  • Upgrade to latest version of argo-ui components
    • Upgrade and force resolve other deps to make sure build passes with latest version
  • Fix intermediate parameters issue with the overflow fix
  • Fix deprecation warnings in build

Modifications

Verification

  • Build passes, install succeeds
  • Very brief spot check on render, see below comment

terrytangyuan and others added 6 commits August 2, 2023 20:46
- need to use `esModuleInterop` to [match Argo UI](https://github.com/argoproj/argo-ui/blob/87d27fb1cb4f6e3ac4a49f85747e471b2efa7512/tsconfig.json#L9) as we are currently importing source TS and not compiled JS
  - `classnames` and `moment` _then_ needed to use default imports instead of namespace imports

Signed-off-by: Anton Gilgur <[email protected]>
- be specific with a SHA
- `@types/uuid` is needed to fix lack of types
  - see also similar commit in Argo CD: argoproj/argo-cd@26d5ad6
    - which is a follow-up to Argo UI: argoproj/argo-ui@06d0e88

Signed-off-by: Anton Gilgur <[email protected]>
- fixes `react-toastify` ESM import errors

- this was pinned to 9.0.8 in argoproj/argo-ui@25663b0, but >9.0.4 seems to require Webpack v5
  - see https://stackoverflow.com/a/72681750/3431180 and related links. these are about CRA, which we don't use, but it appears to affect upstream Webpack, which we do use
  - notably, Argo CD uses [Webpack v5](https://github.com/argoproj/argo-cd/blob/bfaac2b5ac4b96c8af8158902c1b086f7ced7389/ui/package.json#L119) and hence did not see this error

Signed-off-by: Anton Gilgur <[email protected]>
- see https://github.com/foundation/foundation-sites/releases/tag/v6.7.0 which had various upgrades and compat fixes

- no build errors anymore!

Signed-off-by: Anton Gilgur <[email protected]>
Copy link
Member

@isubasinghe isubasinghe left a comment

Choose a reason for hiding this comment

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

Approving it early because CI/UI passes (only really needed to check this). LGTM! Thanks for your work

@agilgur5
Copy link
Contributor Author

agilgur5 commented Aug 15, 2023

Verification

Build passes, install succeeds

Just noting here that I did not do a full test of rendering the UI, as this my first time touching the UI build process so I still need to figure all that out. From a spot test running yarn start, the UI does render, just with some source map warnings for autolinker (a dep that was not changed and is pinned).

There was one piece of the argo-ui upgrade that was particularly noteworthy, a breaking upgrade of antd: argoproj/argo-ui#387 (comment)

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.

LGTM

@terrytangyuan terrytangyuan enabled auto-merge (squash) August 16, 2023 18:43
@terrytangyuan terrytangyuan merged commit 6bccc99 into argoproj:master Aug 16, 2023
23 checks passed
@agilgur5 agilgur5 deleted the upgrade-argo-ui branch August 17, 2023 19:06
@agilgur5 agilgur5 added type/dependencies PRs and issues specific to updating dependencies area/ui javascript Pull requests that update Javascript dependencies labels Aug 20, 2023
@agilgur5
Copy link
Contributor Author

react-toastify forced resolution is removed with build upgrades in #11628

@JPZ13
Copy link
Member

JPZ13 commented Jan 29, 2024

@terrytangyuan @sarabala1979 - did this PR ever get merged in to a v3.4.x commit? I'm looking through v3.4.16 and don't see it

@agilgur5
Copy link
Contributor Author

It's not in the 3.4.x changelog and didn't make it into 3.4.11 which is the only release issue back linked above.

I imagine this one might have had cherry-pick issues due to the yarn.lock changes, but idk

@terrytangyuan
Copy link
Member

If it's not in changelog then there must have been a conflict. See #11648 (comment) and the comment above and this commit is mentioned as not successfully cherry-picked.

Joibel pushed a commit to pipekit/argo-workflows that referenced this pull request Apr 30, 2024
A cherry pick of argoproj#11585 to release/3.4

Changes from argoproj#11585:
* `archived-workflow-details.tsx` changes to `className`
* `yarn.lock` updated, `react-toastify` still pinned to 9.0.3

Co-authored-by: Anton Gilgur <[email protected]>
Co-authored-by: Yuan Tang <[email protected]>
Signed-off-by: Alan Clucas <[email protected]>
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.

UI: Intermediate parameters window is not scrollable
4 participants