fix(deps): upgrade swagger-ui-react v5.17.3, react-dom v18.3.1, and react v18.3.1. Fixes CVEs

[terrytangyuan]

This fixes CVEs: https://security.snyk.io/vuln/SNYK-JS-MICROMATCH-6838728, https://security.snyk.io/vuln/SNYK-JS-BRACES-6838727

   Upgrade swagger-ui-react@5.10.0 to swagger-ui-react@5.17.3 to fix
  ✗ Uncontrolled resource consumption (new) [High Severity][https://security.snyk.io/vuln/SNYK-JS-BRACES-6838727] in braces@3.0.2
    introduced by swagger-ui-react@5.10.0 > patch-package@8.0.0 > find-yarn-workspace-root@2.0.0 > micromatch@4.0.5 > braces@3.0.2
  ✗ Inefficient Regular Expression Complexity (new) [High Severity][https://security.snyk.io/vuln/SNYK-JS-MICROMATCH-6838728] in micromatch@4.0.5
    introduced by swagger-ui-react@5.10.0 > patch-package@8.0.0 > find-yarn-workspace-root@2.0.0 > micromatch@4.0.5

[terrytangyuan]

Followed https://react.dev/blog/2022/03/08/react-18-upgrade-guide

yarn add react react-dom

[terrytangyuan]

Reference: https://react.dev/blog/2022/03/08/react-18-upgrade-guide#updates-to-client-rendering-apis

[agilgur5]

dep overrides aka resolutions

So I mentioned on Slack that this needs a few overrides; upgrading React by two major versions is not what I meant 😕

Notably, I did not update swagger-ui-react past v5.10.x before in #12540 very intentionally due to its compatibility issues.
Since then, the issue I linked there, swagger-api/swagger-ui#9495, was updated with a docs page https://github.com/swagger-api/swagger-ui/wiki/Using-older-version-of-React that we could follow for compatibility. That doc lists what dep overrides are necessary, and that was what I meant.

I mentioned that I hadn't fully tested this locally yet in Slack either -- I was getting this error swagger-api/swagger-ui#9929 at the time, which may require another override.

patch isolation, breaking major updates

As a best practice, we should try and isolate patches, especially security patches, as much as possible, in general.
Bumping another dep by two major versions, especially the foundational UI framework, React, has little isolation to it as it would affect the entire UI either directly or indirectly.

I would suggest that such a foundational, breaking upgrade really get a thorough review by a UI SME as well, which this did not.
The "Verification" section, as is required in the PR template, which is in general important for the UI (where we lack many tests and visual nuances are relevant), especially for such a bump, is missing here as well.

We had not upgraded before due to several incompatibilities. We already had been using deprecated functionality -- off the top of my head, two notable ones I can think of include:

1. refactor(ui): convert WorkflowsList + WorkflowsFilter to functional components #11891 and a few prior refactors of mine removed BasePage, which used the legacy, undocumented context API, which was only promised to work with React 16 and would be removed in future breaking changes.
2. Argo UI still has many class components. In particular, several were using unsafe lifecycle methods which I only recently updated to use the non-deprecated UNSAFE_ prefix in refactor(deps): migrate from archived tslint to eslint argo-ui#509. The deprecated, unprefixed variants were similarly only promised to work with React 16 and were removed in React 17.
   • I also only recently updated argo-ui to a version with those changes in build(ui): improve performance with esbuild-loader #12516. That was merged less than a week before this PR was.

So if upgrading React by two major versions works, it is almost entirely coincidence due to refactoring work I had been doing many months prior to make such an upgrade possible.

Notably, such coincidences mean that there is no way to backport this cleanly.

You also wanted to backport this to Argo 3.4 in #11851 (comment); outside of the issues above, Argo 3.4 is itself only on swagger-ui-react v4 as #12540 was never backported.
EDIT: Actually, Argo 3.5 is also on swagger-ui-react v4 so backporting this to 3.5 per #11997 (comment) is difficult for that reason as well

I hope all the careful refactoring and months-long changes I had been making to prepare for such an upgrade, as well as all the incompatibilities that may result from attempting to backport this PR, illustrate why such breaking upgrades to foundational frameworks need significant diligence & review around them.

CVE has minimal impact, would prefer to remove swagger-ui-react entirely tbh

swagger-ui-react is only used on a single page in the UI, to load the API docs. As of #12061, it isn't even loaded until a user navigates to that page.

As such, a CVE in swagger-ui-react is of minimal effect to Argo users in general.
These particular CVEs are in deps of deps of deps (and so on) of swagger-ui-react and are not really exploitable as such (maybe with a malicious swagger doc, which would require a few other exploits to be able to do, and even then is only a maybe).
I'm also pretty sure they are in devDeps (patch-package is a devDep) and so are false positives too per swagger-api/swagger-ui#8288 (comment) and swagger-api/swagger-ui#9909

Since swagger-ui-react is such a big dep with complex deps of its own (that have also had CVEs more than once, e.g. #12470, #12058) and has various efficiency issues per #6645 et al, I've actually been thinking of entirely removing the API Docs page to reduce our complexity & deps as well as improve the speed of installation and build.
Users can still use the API reference in the docs (which is aided now by versioned docs #11390) and if they want the specific version of their Server, could download it from the API and load it locally in their own editor etc.

IMO, removing the dep/page entirely would be a more stable change to backport than bumping React by two majors.
But we could also ignore the CVE as not exploitable and/or false positive.

[agilgur5]

we should leave the import in the same place; the react deps and the external deps were grouped and alphabetized previously.

Suggested change

	import * as ReactDOM from 'react-dom';
	import {App} from './app';
	import {createRoot} from 'react-dom/client';
	import {createRoot} from 'react-dom/client';
	import {App} from './app';

I've been meaning to add an ESLint or Prettier plugin to auto sort imports for a bit now

[agilgur5]

Suggested change

	const container = document.getElementById('app');
	const root = createRoot(container!);
	const root = createRoot(document.getElementById('app')!);

we can probably simplify this, the container var is otherwise unused

[agilgur5]

The React upgrade also produced a bunch of new install warnings too:

warning " > argo-ui@1.0.0" has incorrect peer dependency "react@^16.9.3".
warning " > argo-ui@1.0.0" has incorrect peer dependency "react-dom@^16.9.3".
warning "argo-ui > react-autocomplete@1.8.1" has incorrect peer dependency "react@^0.14.7 || ^15.0.0-0 || ^16.0.0-0".
warning "argo-ui > react-autocomplete@1.8.1" has incorrect peer dependency "react-dom@^0.14.7 || ^15.0.0-0 || ^16.0.0-0".
warning "argo-ui > react-form@2.16.3" has incorrect peer dependency "react@^15.6.1 || 16".
warning "argo-ui > react-form@2.16.3" has incorrect peer dependency "react-dom@^15.6.1 || 16".
warning "argo-ui > react-form > react-redux@5.1.2" has incorrect peer dependency "react@^0.14.0 || ^15.0.0-0 || ^16.0.0-0".
warning " > react-chartjs-2@2.11.2" has incorrect peer dependency "react@^0.14.0 || ^15.0.0 || ^16.0.0 || ^17.0.0".
warning " > react-chartjs-2@2.11.2" has incorrect peer dependency "react-dom@^0.14.0 || ^15.0.0 || ^16.0.0 || ^17.0.0".

I'm not sure if all of those are safely ignoreable. Some of them are certainly fixable though (react-autocomplete is not, since the library was archived years ago)