Skip to content

Conversation

@david-crespo
Copy link
Collaborator

@david-crespo david-crespo commented Nov 12, 2024

I didn't bother doing all of them, but I did a bunch of the ones that make the route file shorter as a proof of concept. The route config is really verbose and it makes it a bit unwieldy, especially after #2529. This change is better than it looks in terms of line count because we are adding a few lines outside the route config to save lines inside it:

$ git diff --stat main... -- app/routes.tsx
 app/routes.tsx | 285 ++++++++++++++++++++------------------------------------------------
 1 file changed, 82 insertions(+), 203 deletions(-)

However, this change has another big advantage besides looking nice: by conforming to RR's route module API, we can trivially convert <Route {...ProjectsPage}> to <Route lazy={() => import('./pages/ProjectsPage')} to get bundle splitting when and where we want it. This all also will make it easier to convert to the lovely new route config format in React Router v7.

Example use of <Route lazy> image

@vercel
Copy link

vercel bot commented Nov 12, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
console ✅ Ready (Inspect) Visit Preview Nov 12, 2024 5:26pm

loader={SilosPage.loader}
handle={makeCrumb('Silos', pb.silos())}
>
<Route {...SilosPage} handle={makeCrumb('Silos', pb.silos())}>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We could export handle from the module too, but I think that would make it impossible to reason about the crumbs by looking at the config. The components and loaders don’t have this problem.

@david-crespo david-crespo merged commit f3d3810 into main Nov 14, 2024
8 checks passed
@david-crespo david-crespo deleted the route-modules branch November 14, 2024 04:03
david-crespo added a commit to oxidecomputer/omicron that referenced this pull request Nov 14, 2024
david-crespo added a commit to oxidecomputer/omicron that referenced this pull request Nov 14, 2024
oxidecomputer/console@9ef82ba...48fc079

* [48fc079a](oxidecomputer/console@48fc079a)
oxidecomputer/console#2542
* [f3d38103](oxidecomputer/console@f3d38103)
oxidecomputer/console#2545
* [eba6626d](oxidecomputer/console@eba6626d)
bump API for moved system timeseries endpoints
* [6052fdbf](oxidecomputer/console@6052fdbf)
oxidecomputer/console#2549
* [b886e4b8](oxidecomputer/console@b886e4b8)
oxidecomputer/console#2550
* [7303d9d7](oxidecomputer/console@7303d9d7)
oxidecomputer/console#2551
* [e239a8e2](oxidecomputer/console@e239a8e2)
oxidecomputer/console#2540
* [31520a2e](oxidecomputer/console@31520a2e)
move Breadcrumbs component definition into TopBar.tsx
* [ad02edec](oxidecomputer/console@ad02edec)
minor: extract UserMenu component for readability

Co-authored-by: iliana etaoin <iliana@oxide.computer>
@david-crespo david-crespo mentioned this pull request Feb 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants