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

argo-ui: future state of this repo #453

Open
agilgur5 opened this issue Sep 25, 2023 · 9 comments
Open

argo-ui: future state of this repo #453

agilgur5 opened this issue Sep 25, 2023 · 9 comments
Assignees
Labels
type/feature New feature request

Comments

@agilgur5
Copy link
Contributor

agilgur5 commented Sep 25, 2023

This is based on discussions in this Slack thread (which is fairly long and winding) that I had raised after realizing the fairly neglected and incomplete state of this repo when having to do an upgrade of it for Argo Workflows in argoproj/argo-workflows#11585.

This was also mentioned in a Maintainers meeting that I was not part of.

Future State -- intent to phase out(?)

This repo has been neglected for some time and right now there are not that many cross-project contributors. Due to this and the interim state of this repo as not entirely a library (see below), this has resulted in slowing down development while simultaneously not really creating a "consistent look and feel" between projects (such as Workflows and CD). There are some similarities and shared branding, but many differences (in part because CD has received more attention) despite having some shared components.

While it may be possible to improve this situation, some contributors believe that this may not be worthwhile and that a design guide of sorts may be a better substitute. Argoproj also already uses antd as the underlying component library. This repo has higher-level components on top of those as well as some components that are not part of antd. EDIT: Actually this repo itself doesn't use antd per below comment, but newer UIs like Argo Rollouts use antd directly, and CD and Workflows could do the same.
(Personally, I do not really have a strong opinion myself as I am a more recent contributor to Argoproj and am more documenting the current thoughts around this for everyone. I think the "ideal state" of this repo could be great, but if it does not suffice for those purposes and may not ever suffice, while also currently slowing things down, phasing out may certainly make a lot of sense.)

Requiring downstream issues

In any case, as the repo is indeed not really actively maintained (and has no active maintainer list), we should document its current state.

Instead of having contributors create issues and PRs here ad-hoc, it may be better in the interim to require that issues and PRs have a downstream issue in consuming projects like Argo Workflows or Argo CD. That way, maintainers of the consuming projects will be aware of and can review PRs here.

Issue and PR templates can be updated to explicitly note this requirement. Perhaps the README and other docs as well.

TypeScript compatibility issues

I may perhaps be the only one interested in improving this, but as I mentioned in argoproj/argo-workflows#11510 (comment), this repo currently exports raw TSX, instead of compiled JS. This causes compat issues when consuming projects have different TS versions or settings, which means that the settings and version of this repo largely define the settings and version of consuming repos. It also means longer compilation times and that consuming repos must have many of the same devDeps despite not using them directly (which is very confusing and hard to determine -- package managers will not automatically determine this given that this is not how devDeps are meant to be used).

Of particular note is that this repo is on TS 4.9 and does not have the strictest (or even strict, for that matter) type-checking configuration.

This makes it all the more harder to decouple this repo from consuming projects as if they use any components from here, they are hard tied to its version and settings. Compiling the TSX to JS and publishing it, whether in the NPM package or directly in the repo, would significantly improve this situation.

History

This is more of a miscellaneous note, but I think the history of a repo is important context to any decisions.

Argo Workflows

As far as I know, this repo was initially the UI for Argo Workflows. Then it evolved as the Argoproj org evolved, including Argo CD and others. This can be seen in late-2019 PRs #53 and argoproj/argo-workflows#1859 that move the Workflows UI into Argo Workflows. (Notably, COVID-19 started to make its mark on the entire world around that time).

Component Library

Then there was some momentum from contributors to make this repo into a component library for all Argo Projects. Can see some mid-2021 PRs such as #96.

This appears to have never quite been completed though, and this repo has been in this interim state ever since.

v1 vs. v2

I don't know that many details here, but if I'm interpreting correctly, v1 was a first-pass assortment of components and v2 was meant to be "a true component library which is shared between all Argo Projects and published on NPM".

It looks like it was also supposed to modern hooks syntax and other things. There is not a 1-to-1 component match though.

This comment was marked as resolved.

@agilgur5 agilgur5 added the type/feature New feature request label Jan 5, 2024
@agilgur5 agilgur5 self-assigned this Jan 5, 2024
@agilgur5
Copy link
Contributor Author

agilgur5 commented Jan 6, 2024

@terrytangyuan would you mind pinning this issue for higher visibility?

Sorry to have to keep bugging you for assistance; tbh that's probably the main reason I want write permissions at this point, so I can do certain maintenance activities without needing to ask approver+ folks 😅 (like pinning issues, creating and editing labels, etc etc)

This comment was marked as resolved.

@agilgur5
Copy link
Contributor Author

agilgur5 commented May 2, 2024

Jotting down #432 (comment) -- upgrading to Storybook v7 and Webpack v5 is highly non-trivial and may not be worthwhile given the state of this repo

@agilgur5
Copy link
Contributor Author

agilgur5 commented May 11, 2024

While it may be possible to improve this situation, some contributors believe that this may not be worthwhile and that a design guide of sorts may be a better substitute. Argoproj also already uses antd as the underlying component library. This repo has higher-level components on top of those as well as some components that are not part of antd.

Ok apparently antd is actually not used by CD or Workflows or even argo-ui itself as I learned in #553 (comment). The ./antd directory only has Storybook stories and no actual components and is entirely unused.
As I wrote there, Rollouts does use antd directly in various places, and that's probably what CD and Workflows should move to do as well. See that comment for more details.

Given that knowledge, further simplifying this repo by removing all the unused antd code and builds would be good to do on the path to deprecate this repo.

@andrii-korotkov
Copy link

andrii-korotkov commented Jun 11, 2024

Would there be a new tag for this? The current one 2.5.1 is more than 2 years old.

@agilgur5
Copy link
Contributor Author

agilgur5 commented Jun 11, 2024

A new tag for what specifically? CD and Workflows both use a SHA directly as this repo has no release process. I'm also not sure why a deprecated repo would have a new tag?

It's also not clear what your use-case for a tag is, since this is an internal repo

@andrii-korotkov
Copy link

Yeah, not needed. I already got an answer that commit reference is used by projects, which I didn’t know before.

@agilgur5
Copy link
Contributor Author

agilgur5 commented Jul 18, 2024

Jotting down #432 (comment) -- upgrading to Storybook v7 and Webpack v5 is highly non-trivial and may not be worthwhile given the state of this repo

I tried a small bit more on the upgrade as I found another automigration that I didn't know about before, but still had issues. Threw up a draft of some v1 changes fwiw though 🤷 : #568

Also tried to downgrade to fix #469 and that failed spectacularly as well: #567
Given the historical details I found there (lack of working Storybook build for years and incompatible changes by humans suggesting it was not used for dev or testing), I am strongly considering deleting the Storybook entirely to continue reducing the scope of this repo in furtherance of this issue and to leave no confusion about that possibly working. That would remove a good bit of code from this repo as well as lots of deps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/feature New feature request
Projects
None yet
Development

No branches or pull requests

2 participants