AppRootPage: Fixes issue navigating between two app plugin pages (#54519) (#54527)

* AppRootPage: Fixes issue where it was not possible to navigate to another plugin

* Externalize react-router

* fixing test

(cherry picked from commit e5fba788d6)

Co-authored-by: Torkel Ödegaard <torkel@grafana.com>

packages/grafana-toolkit/src/config/webpack.plugin.config.ts

@@ -188,6 +188,7 @@ const getBaseWebpackConfig: WebpackConfigurationGetter = async (options) => {
       'react-redux',
       'redux',
       'rxjs',
+      'react-router',
       'react-router-dom',
       'd3',
       'angular',

public/app/features/plugins/components/AppRootPage.test.tsx

@@ -77,17 +77,18 @@ describe('AppRootPage', () => {
     setEchoSrv(new Echo());
   });
 
+  const pluginMeta = getMockPlugin({
+    id: 'my-awesome-plugin',
+    type: PluginType.app,
+    enabled: true,
+  });
+
   it('should not mount plugin twice if nav is changed', async () => {
     // reproduces https://github.com/grafana/grafana/pull/28105
-
-    getPluginSettingsMock.mockResolvedValue(
-      getMockPlugin({
-        type: PluginType.app,
-        enabled: true,
-      })
-    );
+    getPluginSettingsMock.mockResolvedValue(pluginMeta);
 
     const plugin = new AppPlugin();
+    plugin.meta = pluginMeta;
     plugin.root = RootComponent;
 
     importAppPluginMock.mockResolvedValue(plugin);
@@ -102,12 +103,7 @@ describe('AppRootPage', () => {
   });
 
   it('should not render component if not at plugin path', async () => {
-    getPluginSettingsMock.mockResolvedValue(
-      getMockPlugin({
-        type: PluginType.app,
-        enabled: true,
-      })
-    );
+    getPluginSettingsMock.mockResolvedValue(pluginMeta);
 
     class RootComponent extends Component<AppRootProps> {
       static timesRendered = 0;
@@ -118,6 +114,7 @@ describe('AppRootPage', () => {
     }
 
     const plugin = new AppPlugin();
+    plugin.meta = pluginMeta;
     plugin.root = RootComponent;
 
     importAppPluginMock.mockResolvedValue(plugin);
@@ -127,18 +124,18 @@ describe('AppRootPage', () => {
     expect(await screen.findByText('my great component')).toBeVisible();
 
     // renders the first time
-    expect(RootComponent.timesRendered).toEqual(1);
+    expect(RootComponent.timesRendered).toEqual(2);
 
     await act(async () => {
       locationService.push('/foo');
     });
 
-    expect(RootComponent.timesRendered).toEqual(1);
+    expect(RootComponent.timesRendered).toEqual(2);
 
     await act(async () => {
       locationService.push('/a/my-awesome-plugin');
     });
 
-    expect(RootComponent.timesRendered).toEqual(2);
+    expect(RootComponent.timesRendered).toEqual(4);
   });
 });

public/app/features/plugins/components/AppRootPage.tsx

@@ -94,7 +94,15 @@ class AppRootPage extends Component<Props, State> {
   render() {
     const { loading, plugin, nav, portalNode } = this.state;
 
-    if (plugin && !plugin.root) {
+    if (!plugin || this.props.match.params.pluginId !== plugin.meta.id) {
+      return (
+        <Page>
+          <PageLoader />
+        </Page>
+      );
+    }
+
+    if (!plugin.root) {
       // TODO? redirect to plugin page?
       return <div>No Root App</div>;
     }
@@ -102,15 +110,13 @@ class AppRootPage extends Component<Props, State> {
     return (
       <>
         <InPortal node={portalNode}>
-          {plugin && plugin.root && (
-            <plugin.root
-              meta={plugin.meta}
-              basename={this.props.match.url}
-              onNavChanged={this.onNavChanged}
-              query={this.props.queryParams as KeyValue}
-              path={this.props.location.pathname}
-            />
-          )}
+          <plugin.root
+            meta={plugin.meta}
+            basename={this.props.match.url}
+            onNavChanged={this.onNavChanged}
+            query={this.props.queryParams as KeyValue}
+            path={this.props.location.pathname}
+          />
         </InPortal>
         {nav ? (
           <Page navModel={nav}>