Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Typescript errors #16

Merged
merged 19 commits into from
Jun 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -881,6 +881,18 @@ module.exports = {
},
},

/**
* Enterprise Search overrides
*/
{
files: ['x-pack/plugins/enterprise_search/**/*.{ts,tsx}'],
excludedFiles: ['x-pack/plugins/enterprise_search/**/*.{test,mock}.{ts,tsx}'],
rules: {
'react-hooks/exhaustive-deps': 'off',
'@typescript-eslint/no-explicit-any': 'error',
},
},

/**
* disable jsx-a11y for kbn-ui-framework
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { mockLicenseContext } from './license_context.mock';
*
* const wrapper = mountWithContext(<Component />, { enterpriseSearchUrl: 'someOverride', license: {} });
*/
export const mountWithContext = (children, context) => {
export const mountWithContext = (children: React.ReactNode, context?: object) => {
return mount(
<I18nProvider>
<KibanaContext.Provider value={{ ...mockKibanaContext, ...context }}>
Expand All @@ -40,7 +40,7 @@ export const mountWithContext = (children, context) => {
*
* Same usage/override functionality as mountWithContext
*/
export const mountWithKibanaContext = (children, context) => {
export const mountWithKibanaContext = (children: React.ReactNode, context?: object) => {
return mount(
<KibanaContext.Provider value={{ ...mockKibanaContext, ...context }}>
{children}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,11 @@ const { intl } = intlProvider.getChildContext();
*
* const wrapper = shallowWithIntl(<Component />);
*/
export const shallowWithIntl = (children) => {
return shallow(<I18nProvider>{children}</I18nProvider>, {
context: { intl },
childContextTypes: { intl },
})
export const shallowWithIntl = (children: React.ReactNode) => {
const context = { context: { intl } };

return shallow(<I18nProvider>{children}</I18nProvider>, context)
.childAt(0)
.dive()
.dive(context)
.shallow();
};
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { EngineOverviewHeader } from '../engine_overview_header';

import './empty_states.scss';

export const EmptyState: React.FC<> = () => {
export const EmptyState: React.FC = () => {
const { enterpriseSearchUrl, http } = useContext(KibanaContext) as IKibanaContext;

const buttonProps = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ describe('NoUserState', () => {
});

it('renders with username', () => {
getUserName.mockImplementationOnce(() => 'dolores-abernathy');
(getUserName as jest.Mock).mockImplementationOnce(() => 'dolores-abernathy');

const wrapper = shallowWithIntl(<NoUserState />);
const prompt = wrapper.find(EuiEmptyPrompt).dive();
const description1 = prompt.find(FormattedMessage).at(1).dive();
Expand All @@ -62,7 +63,7 @@ describe('EmptyState', () => {

button.simulate('click');
expect(sendTelemetry).toHaveBeenCalled();
sendTelemetry.mockClear();
(sendTelemetry as jest.Mock).mockClear();
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { EngineOverviewHeader } from '../engine_overview_header';

import './empty_states.scss';

export const ErrorState: ReactFC<> = () => {
export const ErrorState: React.FC = () => {
const { enterpriseSearchUrl } = useContext(KibanaContext) as IKibanaContext;

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { EngineOverviewHeader } from '../engine_overview_header';

import './empty_states.scss';

export const LoadingState: React.FC<> = () => {
export const LoadingState: React.FC = () => {
return (
<EuiPage restrictWidth className="engine-overview empty-state">
<SetBreadcrumbs isRoot />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { getUserName } from '../../utils/get_username';

import './empty_states.scss';

export const NoUserState: React.FC<> = () => {
export const NoUserState: React.FC = () => {
const username = getUserName();

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,15 @@ import '../../../__mocks__/react_router_history.mock';

import React from 'react';
import { act } from 'react-dom/test-utils';
import { render } from 'enzyme';
import { render, ReactWrapper } from 'enzyme';

import { I18nProvider } from '@kbn/i18n/react';
import { KibanaContext } from '../../../';
import { LicenseContext } from '../../../shared/licensing';
import { mountWithContext, mockKibanaContext } from '../../../__mocks__';

import { EmptyState, ErrorState, NoUserState } from '../empty_states';
import { EngineTable } from './engine_table';
import { EngineTable, IEngineTablePagination } from './engine_table';

import { EngineOverview } from './';

Expand All @@ -25,7 +25,7 @@ describe('EngineOverview', () => {
it('isLoading', () => {
// We use render() instead of mount() here to not trigger lifecycle methods (i.e., useEffect)
// TODO: Consider pulling this out to a renderWithContext mock/helper
const wrapper = render(
const wrapper: Cheerio = render(

Choose a reason for hiding this comment

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

Why don't we have to import Cheerio?

Copy link
Owner Author

@cee-chen cee-chen Jun 4, 2020

Choose a reason for hiding this comment

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

Short answer: I have no idea lmao :dead_inside:

Long answer: I think it's a global declared by either Jest or Enzyme. This threw me for a massive loop as well and I spent a good amount of time trying to figure out WTF wrapper type it wanted. I tried ReactWrapper & ShallowWrapper (threw type errors). Enzyme's docs lists render() returning a CheerioWrapper (even though the URL structure is nested under ShallowWrapper??) but every time I tried to import CheerioWrapper from Enzyme I got a "export does not exist error".

I eventually tried YOLOing this and it worked and had also spent enough time on it that just moved on 🤦‍♀️ Anyway, this is why people avoid render() haha - it has super weird behavior/second class docs compared to shallow/mount

<I18nProvider>
<KibanaContext.Provider value={{ http: {} }}>
<LicenseContext.Provider value={{ license: {} }}>
Expand Down Expand Up @@ -85,7 +85,7 @@ describe('EngineOverview', () => {
},
};
const mockApi = jest.fn(() => mockedApiResponse);
let wrapper;
let wrapper: ReactWrapper;

beforeAll(async () => {
wrapper = await mountWithApiMock({ get: mockApi });
Expand All @@ -105,7 +105,8 @@ describe('EngineOverview', () => {
});

describe('pagination', () => {
const getTablePagination = () => wrapper.find(EngineTable).first().prop('pagination');
const getTablePagination: () => IEngineTablePagination = () =>
wrapper.find(EngineTable).first().prop('pagination');

it('passes down page data from the API', () => {
const pagination = getTablePagination();
Expand Down Expand Up @@ -156,8 +157,8 @@ describe('EngineOverview', () => {
* Test helpers
*/

const mountWithApiMock = async ({ get, license }) => {
let wrapper;
const mountWithApiMock = async ({ get, license }: { get(): any; license?: object }) => {
let wrapper: ReactWrapper | undefined;
const httpMock = { ...mockKibanaContext.http, get };

// We get a lot of act() warning/errors in the terminal without this.
Expand All @@ -166,8 +167,12 @@ describe('EngineOverview', () => {
await act(async () => {
wrapper = mountWithContext(<EngineOverview />, { http: httpMock, license });
});
wrapper.update(); // This seems to be required for the DOM to actually update
if (wrapper) {
wrapper.update(); // This seems to be required for the DOM to actually update

return wrapper;
return wrapper;
} else {
throw new Error('Could not mount wrapper');
}
};
});
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import { EngineTable } from './engine_table';

import './engine_overview.scss';

export const EngineOverview: ReactFC<> = () => {
export const EngineOverview: React.FC = () => {
const { http } = useContext(KibanaContext) as IKibanaContext;
const { license } = useContext(LicenseContext) as ILicenseContext;

Expand All @@ -45,12 +45,12 @@ export const EngineOverview: ReactFC<> = () => {
const [metaEnginesPage, setMetaEnginesPage] = useState(1);
const [metaEnginesTotal, setMetaEnginesTotal] = useState(0);

const getEnginesData = async ({ type, pageIndex }) => {
const getEnginesData = async ({ type, pageIndex }: IGetEnginesParams) => {
return await http.get('/api/app_search/engines', {
query: { type, pageIndex },
});
};
const setEnginesData = async (params, callbacks) => {
const setEnginesData = async (params: IGetEnginesParams, callbacks: ISetEnginesCallbacks) => {
try {
const response = await getEnginesData(params);

Expand All @@ -72,7 +72,7 @@ export const EngineOverview: ReactFC<> = () => {
const callbacks = { setResults: setEngines, setResultsTotal: setEnginesTotal };

setEnginesData(params, callbacks);
}, [enginesPage]); // eslint-disable-line react-hooks/exhaustive-deps
}, [enginesPage]);

useEffect(() => {
if (hasPlatinumLicense(license)) {
Expand All @@ -81,7 +81,7 @@ export const EngineOverview: ReactFC<> = () => {

setEnginesData(params, callbacks);
}
}, [license, metaEnginesPage]); // eslint-disable-line react-hooks/exhaustive-deps
}, [license, metaEnginesPage]);

if (hasErrorConnecting) return <ErrorState />;
if (hasNoAccount) return <NoUserState />;
Expand Down Expand Up @@ -150,3 +150,16 @@ export const EngineOverview: ReactFC<> = () => {
</EuiPage>
);
};

/**
* Type definitions
*/

interface IGetEnginesParams {
type: string;
pageIndex: number;
}
interface ISetEnginesCallbacks {
setResults: React.Dispatch<React.SetStateAction<never[]>>;
setResultsTotal: React.Dispatch<React.SetStateAction<number>>;
}
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,9 @@ describe('EngineTable', () => {

it('handles empty data', () => {
const emptyWrapper = mountWithContext(
<EngineTable data={[]} pagination={{ totalEngines: 0 }} />
<EngineTable data={[]} pagination={{ totalEngines: 0, pageIndex: 0, onPaginate: () => {} }} />
);
const emptyTable = wrapper.find(EuiBasicTable);
const emptyTable = emptyWrapper.find(EuiBasicTable);
expect(emptyTable.prop('pagination').pageIndex).toEqual(0);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

import React, { useContext } from 'react';
import { EuiBasicTable, EuiLink } from '@elastic/eui';
import { EuiBasicTable, EuiBasicTableColumn, EuiLink } from '@elastic/eui';
import { FormattedMessage, FormattedDate, FormattedNumber } from '@kbn/i18n/react';
import { i18n } from '@kbn/i18n';

Expand All @@ -14,31 +14,33 @@ import { KibanaContext, IKibanaContext } from '../../../index';

import { ENGINES_PAGE_SIZE } from '../../../../../common/constants';

interface IEngineTableProps {
data: Array<{
name: string;
created_at: string;
document_count: number;
field_count: number;
}>;
pagination: {
totalEngines: number;
pageIndex: number;
onPaginate(pageIndex: number);
};
export interface IEngineTableData {
name: string;
created_at: string;
document_count: number;
field_count: number;
}
export interface IEngineTablePagination {
totalEngines: number;
pageIndex: number;
onPaginate(pageIndex: number): void;
}
export interface IEngineTableProps {
data: IEngineTableData[];
pagination: IEngineTablePagination;
}
interface IOnChange {
export interface IOnChange {
page: {
index: number;
};
}

export const EngineTable: ReactFC<IEngineTableProps> = ({
export const EngineTable: React.FC<IEngineTableProps> = ({
data,
pagination: { totalEngines, pageIndex = 0, onPaginate },
}) => {
const { enterpriseSearchUrl, http } = useContext(KibanaContext) as IKibanaContext;
const engineLinkProps = (name) => ({
const engineLinkProps = (name: string) => ({
href: `${enterpriseSearchUrl}/as/engines/${name}`,
target: '_blank',
onClick: () =>
Expand All @@ -50,13 +52,13 @@ export const EngineTable: ReactFC<IEngineTableProps> = ({
}),
});

const columns = [
const columns: Array<EuiBasicTableColumn<IEngineTableData>> = [
{
field: 'name',
name: i18n.translate('xpack.appSearch.enginesOverview.table.column.name', {
defaultMessage: 'Name',
}),
render: (name) => (
render: (name: string) => (
<EuiLink data-test-subj="engineNameLink" {...engineLinkProps(name)}>
{name}
</EuiLink>
Expand All @@ -65,6 +67,8 @@ export const EngineTable: ReactFC<IEngineTableProps> = ({
truncateText: true,
mobileOptions: {
header: true,
// Note: the below props are valid props per https://elastic.github.io/eui/#/tabular-content/tables (Responsive tables), but EUI's types have a bug reporting it as an error
// @ts-ignore
enlarge: true,
fullWidth: true,
truncateText: false,
Expand All @@ -76,7 +80,7 @@ export const EngineTable: ReactFC<IEngineTableProps> = ({
defaultMessage: 'Created At',
}),
dataType: 'string',
render: (dateString) => (
render: (dateString: string) => (
// e.g., January 1, 1970
<FormattedDate value={new Date(dateString)} year="numeric" month="long" day="numeric" />
),
Expand All @@ -87,7 +91,7 @@ export const EngineTable: ReactFC<IEngineTableProps> = ({
defaultMessage: 'Document Count',
}),
dataType: 'number',
render: (number) => <FormattedNumber value={number} />,
render: (number: number) => <FormattedNumber value={number} />,
truncateText: true,
},
{
Expand All @@ -96,7 +100,7 @@ export const EngineTable: ReactFC<IEngineTableProps> = ({
defaultMessage: 'Field Count',
}),
dataType: 'number',
render: (number) => <FormattedNumber value={number} />,
render: (number: number) => <FormattedNumber value={number} />,
truncateText: true,
},
{
Expand All @@ -105,7 +109,7 @@ export const EngineTable: ReactFC<IEngineTableProps> = ({
defaultMessage: 'Actions',
}),
dataType: 'string',
render: (name) => (
render: (name: string) => (
<EuiLink {...engineLinkProps(name)}>
<FormattedMessage
id="xpack.appSearch.enginesOverview.table.action.manage"
Expand All @@ -128,7 +132,7 @@ export const EngineTable: ReactFC<IEngineTableProps> = ({
totalItemCount: totalEngines,
hidePerPageOptions: true,
}}
onChange={({ page }): IOnChange => {
onChange={({ page }: IOnChange) => {
const { index } = page;
onPaginate(index + 1); // Note on paging - App Search's API pages start at 1, EuiBasicTables' pages start at 0
}}
Expand Down
Loading