From 97526fc492c0b3137d30fcf521139f1a2734ec7b Mon Sep 17 00:00:00 2001 From: Domas Date: Wed, 21 Oct 2020 10:58:23 +0300 Subject: [PATCH] Plugins: do not remount app plugin on nav change (#28105) * do not remount app plugin on nav change * test for not mounting app plugin twice --- package.json | 1 + .../app/features/plugins/AppRootPage.test.tsx | 113 ++++++++++++++++++ public/app/features/plugins/AppRootPage.tsx | 33 +++-- yarn.lock | 5 + 4 files changed, 139 insertions(+), 13 deletions(-) create mode 100644 public/app/features/plugins/AppRootPage.test.tsx diff --git a/package.json b/package.json index a2beafa0350..4352259db41 100644 --- a/package.json +++ b/package.json @@ -269,6 +269,7 @@ "react-loadable": "5.5.0", "react-popper": "1.3.3", "react-redux": "7.2.0", + "react-reverse-portal": "^2.0.1", "react-sizeme": "2.6.12", "react-split-pane": "0.1.89", "react-transition-group": "4.3.0", diff --git a/public/app/features/plugins/AppRootPage.test.tsx b/public/app/features/plugins/AppRootPage.test.tsx new file mode 100644 index 00000000000..44557ab5dd1 --- /dev/null +++ b/public/app/features/plugins/AppRootPage.test.tsx @@ -0,0 +1,113 @@ +import { render, screen } from '@testing-library/react'; +import React, { Component } from 'react'; +import { StoreState } from 'app/types'; +import { Provider } from 'react-redux'; +import configureStore from 'redux-mock-store'; +import AppRootPage from './AppRootPage'; +import { getPluginSettings } from './PluginSettingsCache'; +import { importAppPlugin } from './plugin_loader'; +import { getMockPlugin } from './__mocks__/pluginMocks'; +import { AppPlugin, PluginType, AppRootProps, NavModelItem } from '@grafana/data'; + +jest.mock('./PluginSettingsCache', () => ({ + getPluginSettings: jest.fn(), +})); +jest.mock('./plugin_loader', () => ({ + importAppPlugin: jest.fn(), +})); + +const importAppPluginMock = importAppPlugin as jest.Mock< + ReturnType, + Parameters +>; + +const getPluginSettingsMock = getPluginSettings as jest.Mock< + ReturnType, + Parameters +>; + +const initialState: Partial = { + location: { + routeParams: { + pluginId: 'my-awesome-plugin', + slug: 'my-awesome-plugin', + }, + query: {}, + path: '/a/my-awesome-plugin', + url: '', + replace: false, + lastUpdated: 1, + }, +}; + +function renderWithStore(soreState: Partial = initialState) { + const store = configureStore()(soreState as StoreState); + render( + + + + ); + return store; +} + +describe('AppRootPage', () => { + beforeEach(() => { + jest.resetAllMocks(); + }); + + 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, + }) + ); + + let timesMounted = 0; + + // a very basic component that does what most plugins do: + // immediately update nav on mounting + class RootComponent extends Component { + componentDidMount() { + timesMounted++; + const node: NavModelItem = { + text: 'My Great plugin', + children: [ + { + text: 'A page', + url: '/apage', + id: 'a', + }, + { + text: 'Another page', + url: '/anotherpage', + id: 'b', + }, + ], + }; + this.props.onNavChanged({ + main: node, + node, + }); + } + render() { + return

my great plugin

; + } + } + + const plugin = new AppPlugin(); + plugin.root = RootComponent; + + importAppPluginMock.mockResolvedValue(plugin); + + renderWithStore(); + + // check that plugin and nav links were rendered, and plugin is mounted only once + await screen.findByText('my great plugin'); + await screen.findByRole('link', { name: /A page/ }); + await screen.findByRole('link', { name: /Another page/ }); + expect(timesMounted).toEqual(1); + }); +}); diff --git a/public/app/features/plugins/AppRootPage.tsx b/public/app/features/plugins/AppRootPage.tsx index 47ef1a6702a..2db6db587c1 100644 --- a/public/app/features/plugins/AppRootPage.tsx +++ b/public/app/features/plugins/AppRootPage.tsx @@ -5,6 +5,7 @@ import { connect } from 'react-redux'; // Types import { StoreState } from 'app/types'; import { AppEvents, AppPlugin, AppPluginMeta, NavModel, PluginType, UrlQueryMap } from '@grafana/data'; +import { createHtmlPortalNode, InPortal, OutPortal, HtmlPortalNode } from 'react-reverse-portal'; import Page from 'app/core/components/Page/Page'; import { getPluginSettings } from './PluginSettingsCache'; @@ -22,6 +23,7 @@ interface Props { interface State { loading: boolean; + portalNode: HtmlPortalNode; plugin?: AppPlugin | null; nav?: NavModel; } @@ -44,6 +46,7 @@ class AppRootPage extends Component { super(props); this.state = { loading: true, + portalNode: createHtmlPortalNode(), }; } @@ -76,29 +79,33 @@ class AppRootPage extends Component { render() { const { path, query } = this.props; - const { loading, plugin, nav } = this.state; + const { loading, plugin, nav, portalNode } = this.state; if (plugin && !plugin.root) { // TODO? redirect to plugin page? return
No Root App
; } - // When no naviagion is set, give full control to the app plugin - if (!nav) { - if (plugin && plugin.root) { - return ; - } - return ; - } - return ( - - + <> + {plugin && plugin.root && ( )} - - + + {nav ? ( + + + + + + ) : ( + <> + + {loading && } + + )} + ); } } diff --git a/yarn.lock b/yarn.lock index 95ca2d4acad..be109c548f5 100644 --- a/yarn.lock +++ b/yarn.lock @@ -22659,6 +22659,11 @@ react-resizable@^1.9.0: prop-types "15.x" react-draggable "^4.0.3" +react-reverse-portal@^2.0.1: + version "2.0.1" + resolved "https://registry.yarnpkg.com/react-reverse-portal/-/react-reverse-portal-2.0.1.tgz#23b18292c531fb7b343d85a614c15a995838ba31" + integrity sha512-sj/D9nSHspqV8i8hWkTSZ5Ohnrqk2A5fkDKw4Xe/zV4OfF1UYwmbzrxLdmNRdKkWgQwnXIxaa2E3FC7QYdZAeA== + react-select@^3.0.8: version "3.0.8" resolved "https://registry.yarnpkg.com/react-select/-/react-select-3.0.8.tgz#06ff764e29db843bcec439ef13e196865242e0c1"