Skip to content

Commit

Permalink
Fix react hooks ordering bug with license status updates and fix test…
Browse files Browse the repository at this point in the history
… (wait for first license object before rendering)
  • Loading branch information
jloleysens committed Jan 21, 2020
1 parent 5994f61 commit c14047c
Show file tree
Hide file tree
Showing 7 changed files with 40 additions and 30 deletions.
3 changes: 2 additions & 1 deletion x-pack/plugins/searchprofiler/public/application/boot.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,16 @@
import { render, unmountComponentAtNode } from 'react-dom';
import { HttpStart as Http, ToastsSetup } from 'kibana/public';
import React from 'react';

import { LicenseStatus } from '../../common/types';
import { App } from '.';

export interface Dependencies {
el: HTMLElement;
http: Http;
getLicenseStatus: () => LicenseStatus;
I18nContext: any;
notifications: ToastsSetup;
initialLicenseStatus: LicenseStatus;
}

export type AppDependencies = Omit<Dependencies, 'el'>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export const LicenseWarningNotice = () => {
title={i18n.translate('xpack.searchProfiler.licenseErrorMessageTitle', {
defaultMessage: 'License error',
})}
color="warning"
color="danger"
iconType="alert"
style={{ padding: '16px' }}
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,17 @@
* you may not use this file except in compliance with the Elastic License.
*/

import React, { useContext, createContext } from 'react';
import React, { useContext, createContext, useCallback } from 'react';

import { HttpSetup, ToastsSetup } from 'kibana/public';
import { LicenseStatus } from '../../../common/types';

export interface ContextArgs {
http: HttpSetup;
notifications: ToastsSetup;
initialLicenseStatus: LicenseStatus;
}

export interface ContextValue {
http: HttpSetup;
notifications: ToastsSetup;
Expand All @@ -18,12 +25,24 @@ const AppContext = createContext<ContextValue>(null as any);

export const AppContextProvider = ({
children,
value,
args: { http, notifications, initialLicenseStatus },
}: {
children: React.ReactNode;
value: ContextValue;
args: ContextArgs;
}) => {
return <AppContext.Provider value={value}>{children}</AppContext.Provider>;
const getLicenseStatus = useCallback(() => initialLicenseStatus, [initialLicenseStatus]);

return (
<AppContext.Provider
value={{
http,
notifications,
getLicenseStatus,
}}
>
{children}
</AppContext.Provider>
);
};

export const useAppContext = () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,13 @@ export const Editor = memo(({ licenseEnabled, initialValue, onEditorReady }: Pro

const [textArea, setTextArea] = useState<HTMLTextAreaElement | null>(null);

if (licenseEnabled) {
useUIAceKeyboardMode(textArea);
}
useUIAceKeyboardMode(textArea);

useEffect(() => {
const divEl = containerRef.current;
editorInstanceRef.current = initializeEditor({ el: divEl, licenseEnabled });
editorInstanceRef.current.setValue(initialValue, 1);
setTextArea(containerRef.current!.querySelector('textarea'));
setTextArea(licenseEnabled ? containerRef.current!.querySelector('textarea') : null);

onEditorReady(createEditorShim(editorInstanceRef.current));
}, [initialValue, onEditorReady, licenseEnabled]);
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/searchprofiler/public/application/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ import { ProfileContextProvider } from './contexts/profiler_context';

import { AppDependencies } from './boot';

export function App({ I18nContext, getLicenseStatus, notifications, http }: AppDependencies) {
export function App({ I18nContext, initialLicenseStatus, notifications, http }: AppDependencies) {
return (
<I18nContext>
<AppContextProvider value={{ getLicenseStatus, notifications, http }}>
<AppContextProvider args={{ initialLicenseStatus, notifications, http }}>
<ProfileContextProvider>
<Main />
</ProfileContextProvider>
Expand Down
26 changes: 9 additions & 17 deletions x-pack/plugins/searchprofiler/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,16 @@

import { i18n } from '@kbn/i18n';
import { Plugin, CoreStart, CoreSetup, PluginInitializerContext } from 'kibana/public';
import { first } from 'rxjs/operators';

import { FeatureCatalogueCategory } from '../../../../src/plugins/home/public';
import { LICENSE_CHECK_STATE } from '../../licensing/public';

import { PLUGIN } from '../common/constants';
import { AppPublicPluginDependencies } from './types';
import { LicenseStatus } from '../common/types';

export class SearchProfilerUIPlugin implements Plugin<void, void, AppPublicPluginDependencies> {
private licenseStatus: LicenseStatus;

constructor(ctx: PluginInitializerContext) {
this.licenseStatus = { valid: false };
}
constructor(ctx: PluginInitializerContext) {}

async setup(
{ http, getStartServices }: CoreSetup,
Expand Down Expand Up @@ -50,25 +46,21 @@ export class SearchProfilerUIPlugin implements Plugin<void, void, AppPublicPlugi
const [coreStart] = await getStartServices();
const { notifications, i18n: i18nDep } = coreStart;
const { boot } = await import('./application/boot');

const license = await licensing.license$.pipe(first()).toPromise();
const { state, message } = license.check(PLUGIN.id, PLUGIN.minimumLicenseType);
const initialLicenseStatus =
state === LICENSE_CHECK_STATE.Valid ? { valid: true } : { valid: false, message };

return boot({
http,
getLicenseStatus: () => this.licenseStatus,
initialLicenseStatus,
el: params.element,
I18nContext: i18nDep.Context,
notifications: notifications.toasts,
});
},
});

licensing.license$.subscribe(license => {
const { state, message } = license.check(PLUGIN.id, PLUGIN.minimumLicenseType);
const isAvailable = state === LICENSE_CHECK_STATE.Valid;
if (isAvailable) {
this.licenseStatus = { valid: true };
} else {
this.licenseStatus = { valid: false, message };
}
});
}

async start(core: CoreStart, plugins: any) {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export default function({ getPageObjects, getService }: FtrProviderContext) {
await aceEditor.setValue(editorTestSubjectSelector, input);

await retry.waitFor(
`parser errors to match expection: HAS ${expectation ? 'ERRORS' : 'NO ERRORS'}`,
`parser errors to match expectation: HAS ${expectation ? 'ERRORS' : 'NO ERRORS'}`,
async () => {
const actual = await aceEditor.hasParseErrors(editorTestSubjectSelector);
return expectation === actual;
Expand Down

0 comments on commit c14047c

Please sign in to comment.