Fix: Routing bug on uninitialised plugin

[ckbedwell]

Problem

When the plugin is uninitialised, when clicking different routes the screen blinks with rendering the route you clicked on before being redirected to the home page. The same behaviour happens for the un-provisioned state of the plugin.

Screen.Recording.2023-10-13.at.15.24.46.mov

Solution

I've removed the Setup and Provisioned routes and simply return early so that the other routes don't exist yet. I've added a useLayoutEffect to reroute back to home so that the breadcrumbs and side nav display correctly. I've had to put navigate in a useCallback so it is stable as without that it ends up an infinite render. I can't see any adverse effects this would have?

Screen.Recording.2023-10-13.at.15.51.54.mov

Additional notes

Please tell me if I am being too heavy handed in some of my refactoring! I've taken a few liberties as I thought these changes would make things easier to read and be more consistent. These include:

• I've renamed and relocated Alerting to AlertingPage. It was in the components folder but it wasn't used elsewhere, so I figured it made sense to rename and relocate it.

• I've cleaned up the Routing component so that the routes are easier to read. It felt slightly arbitrary that some routes had context providers / PluginPage wrappers in this component vs their own component, so I've moved them all to their own components for consistency.

• The provisioning global state was only used in routing and it was derived from meta. So I've removed it and I've just pulled meta directly from the context rather than passing it down as a prop.

• Changing default exports to named exports for consistency. If we end up using webpack chunks for code splitting some things will need to be default exports but that feature isn't used currently.

• I've started reordering the imports as I was finding them difficult to skim through and find what I was looking for. We could add an eslint rule for this but the order I am aiming for is something like:

• Node modules

  • react-related things
  • styling-related things
  • grafana-related things
  • everything else

• Empty line

• Repo types

• Context

• Utilities

• Hooks

• Components

I find doing it like this means I can find what I'm looking for. What do you think?

I also noticed that the plugin does have a dependency on Grafana 10 but there are some cases where we are referring to pre-Grafana 10 features (references to topNav feature flag, as one example), are we free to remove them to clean up code or should they be kept?

[rdubrock]

I also noticed that the plugin does have a dependency on Grafana 10 but there are some cases where we are referring to pre-Grafana 10 features (references to topNav feature flag, as one example), are we free to remove them to clean up code or should they be kept?

It's a bit tricky to decide when to remove them. I think at this point, we've tipped over the point of no longer being backwards compatible. I generally try to stay backwards compatible as long as possible. Up until recently it was compatible back to Grafana v8, but I had some gross dependencies (d3) that I could get rid of by moving to scenes, so I ripped that bandaid off recently.

[rdubrock]

Nice! Looks great

[rdubrock]

Merge at your leisure!