Open AlanGreene opened 1 year ago
Recent React Router v5 releases have some fixes for React 18 compatibility so we should try updating React first and see how that goes.
Our previous attempt at updating React Router using the compatibility layer ran into some issues but with most other dependencies now updated, and assuming the React 18 update works as expected, it's time to try this again.
If the compatibility layer doesn't work out we can just update it all in one go but it would be nice to break it down into a series of smaller changes if possible to facilitate easier review and testing.
PR #2542 opened to add the compatibility layer and start migrating to the new API. All routes have been converted to use the compatibility layer, including hooks from the new API.
Remaining:
<Route children>
to <Route element>
children
render prop approach is no longer supported to always render content whether the path matches or not. Instead we duplicate the route (as recommended in the docs) including a 'splat' (wildcard) for any unmatched paths to ensure it always renders even when on a non-namespaced path.<Switch>
becomes <Routes>
exact
prop on routes<Link>
to address the component
prop issueuseSearchParams
instead of current manual approach using URLSearchParams
etc./lifecycle frozen Freezing so it doesn't get auto-closed due to inactivity. This is a longer term item that's currently blocked on the move to React 18, Carbon 11, and related changes.
Revisiting this in the near future. According to the declared dependencies we should actually be able to upgrade to React Router v6 while remaining on React 17. Perhaps some of the previous issues were red herrings or have since been resolved 🤞
$ npm show react-router-dom version
6.8.1
$ npm show react-router-dom@6.8.1 peerDependencies
{ react: '>=16.8', 'react-dom': '>=16.8' }
Last PR related to compatibility layer issues: https://github.com/tektoncd/dashboard/pull/2650 May be resolved now
I've been looking at a large set of updates across our main dependencies. There are a number of improvements in React Router 6 that allow us to remove previous workarounds / custom components that were added purely for managing / interacting with the router, in favour of built-in APIs now provided by RR.
These allow us to significantly simplify the client architecture, reduce maintenance overhead, and further lower the barrier to entry for new contributors. As a result it also makes some previously explored feature requests more feasible.
More to come after our next LTS release is done so we can focus more effort on these upgrades.
Once https://github.com/tektoncd/dashboard/pull/3465 is merged we can start on simplifying and unifying most of the per-resource type containers. This means that for most resources they'll just need their routes defined, and everything else should be handled without the need for additional code.
Highly custom pages such as those for PipelineRuns, TaskRuns, etc. will keep their custom containers for now, but in future should also be migrated to the common containers as wrappers, with custom render functions as needed.
Also tracking React Query (TanStack Query) update to v5:
cacheTime
to gcTime
isLoading
to isPending
status: loading
to status: pending
, and isInitialLoading
to isLoading
isLoading
for most of our use cases? (the new isLoading
is isPending && isFetching
)
Update react-router / react-router-dom to the v6 releases. This will likely be a breaking change for consumers of the Dashboard's
@tektoncd/dashboard-components
and@tektoncd/dashboard-utils
packages and will need to be managed carefully.See the documentation for details of the changes: https://reactrouter.com/en/main/upgrading/v5
There is a compatibility layer that should make the update process a bit smoother, allowing us to use both APIs in parallel while we make the required changes across the app, although it may not cover all use cases. This is documented at: https://github.com/remix-run/react-router/discussions/8753
Some changes have already been done in preparation for this update:
Some additional changes identified:
component
prop is no longer supported in v6,useHref
hook in custom component insteaduseSearchParams
instead of current manualURLSearchParams
andhistory.push
approach. Not 'required' but is a nice cleanup that we should considerOnce we agree a timeline for this update the next steps will be to review the latest documentation, enable the compatibility layer, and start by updating a small set of routes to validate the process.