From 4e9ea81708c5b924695890535bc17cdfd64b38bc Mon Sep 17 00:00:00 2001 From: Chris Williams Date: Thu, 13 Dec 2018 17:15:23 -0800 Subject: [PATCH] [build] fix typescript builds (#56) * [build] fix typescript builds * [typescript] ensure types pass in build * [typescript][connection] declare modules in tests * [typescript][connection] fix ts errors in tests * [typescript][connection] test/types.ts => types/external.d.ts * [chart][typescript] add @types/react-loadable * [chart][components] convert to ts * [charts][tests][broken] convert to ts * [chart][typescript] re-write component generics * [chart][typescript] fix reactify generic, add react-dom types * [chart][typescript] more iteration * - Tweaking reactify types (using Readonly types). - Uncovered an issue in which ReactifyProps and Props can collide on id and className. - Move @types/react-loadable to dev dependency - Fixing a lint error * [chart][deps] add @types/fetch-mock * [client][typescript] add and export SupersetClientInterface * [chart][clients] fix ts * [charts][components] more ts iterations * [chart][client] assert FormData type * [chart][deps] try adding newest @types/react * [chart][components][ts] fix reactify prop TS * [chart] lint * [chart][ts] lint #2, move @types to deps not dev-deps * [chart][jest] fix tests * [chart][tests] up branch coverage * [chart][ts][test] null => undefined * [chart][tests] hundo * [chart][tests] update name * [chart][ts] ChartClient type fixes --- package.json | 13 +-- packages/superset-ui-chart/package.json | 3 + .../src/clients/ChartClient.ts | 40 ++++----- .../src/components/createLoadableRenderer.js | 47 ----------- .../src/components/createLoadableRenderer.ts | 52 ++++++++++++ .../src/components/reactify.jsx | 51 ------------ .../src/components/reactify.tsx | 82 +++++++++++++++++++ .../src/{index.js => index.ts} | 0 .../superset-ui-chart/src/query/FormData.ts | 6 +- .../test/clients/ChartClient.test.ts | 52 ++++++------ ...st.jsx => createLoadableRenderer.test.tsx} | 23 ++++-- .../{reactify.test.jsx => reactify.test.tsx} | 29 ++++--- .../test/models/ChartPlugin.test.ts | 24 +++--- .../src/SupersetClient.ts | 17 +++- .../src/callApi/parseResponse.ts | 6 +- packages/superset-ui-connection/src/index.ts | 2 +- packages/superset-ui-connection/src/types.ts | 2 +- .../test/SupersetClientClass.test.ts | 19 +++-- .../test/callApi/callApi.test.ts | 6 +- .../test/callApi/parseResponse.test.ts | 4 +- .../test/utils/throwIfCalled.ts | 2 +- .../types/external.d.ts | 1 + .../factories/createD3TimeFormatter.test.js | 1 + 23 files changed, 273 insertions(+), 209 deletions(-) delete mode 100644 packages/superset-ui-chart/src/components/createLoadableRenderer.js create mode 100644 packages/superset-ui-chart/src/components/createLoadableRenderer.ts delete mode 100644 packages/superset-ui-chart/src/components/reactify.jsx create mode 100644 packages/superset-ui-chart/src/components/reactify.tsx rename packages/superset-ui-chart/src/{index.js => index.ts} (100%) rename packages/superset-ui-chart/test/components/{createLoadableRenderer.test.jsx => createLoadableRenderer.test.tsx} (82%) rename packages/superset-ui-chart/test/components/{reactify.test.jsx => reactify.test.tsx} (71%) create mode 100644 packages/superset-ui-connection/types/external.d.ts diff --git a/package.json b/package.json index 59efae32d..a81d294fd 100644 --- a/package.json +++ b/package.json @@ -5,10 +5,11 @@ "private": true, "scripts": { "bootstrap": "lerna bootstrap", - "build": "yarn run build:cjs && yarn run build:esm && yarn run build:ts", - "build:cjs": "NODE_ENV=production beemo babel ./src --out-dir lib/ --minify --workspaces=\"@superset-ui/!(demo|generator-superset)\"", - "build:esm": "NODE_ENV=production beemo babel ./src --out-dir esm/ --esm --minify --workspaces=\"@superset-ui/!(demo|generator-superset)\"", - "build:ts": "NODE_ENV=production beemo typescript --workspaces=\"@superset-ui/(connection|chart)\"", + "build": "yarn run build:cjs && yarn run build:esm && yarn run type:dts", + "build:cjs": "NODE_ENV=production beemo babel --extensions=\".js,.jsx,.ts,.tsx\" ./src --out-dir lib/ --minify --workspaces=\"@superset-ui/!(demo|generator-superset)\"", + "build:esm": "NODE_ENV=production beemo babel --extensions=\".js,.jsx,.ts,.tsx\" ./src --out-dir esm/ --esm --minify --workspaces=\"@superset-ui/!(demo|generator-superset)\"", + "type": "NODE_ENV=production beemo typescript --workspaces=\"@superset-ui/(connection|chart)\" --noEmit", + "type:dts": "NODE_ENV=production beemo typescript --workspaces=\"@superset-ui/(connection|chart)\" --emitDeclarationOnly", "lint": "beemo create-config prettier && beemo eslint \"./packages/*/{src,test,storybook}/**/*.{js,jsx,ts,tsx}\"", "lint:fix": "beemo create-config prettier && beemo eslint --fix \"./packages/*/{src,test,storybook}/**/*.{js,jsx,ts,tsx}\"", "jest": "beemo jest --color --coverage --react", @@ -18,7 +19,7 @@ "pretest": "yarn run lint", "prettier": "beemo prettier \"./packages/*/{src,test,storybook}/**/*.{js,jsx,ts,tsx,json,md}\"", "release": "yarn run prepare-release && lerna publish && yarn run postrelease", - "test": "yarn run jest", + "test": "yarn run type && yarn run jest", "test:watch": "beemo create-config jest --react && jest --watch" }, "repository": "https://github.com/apache-superset/superset-ui.git", @@ -36,7 +37,7 @@ ], "license": "Apache-2.0", "devDependencies": { - "@data-ui/build-config": "^0.0.33", + "@data-ui/build-config": "^0.0.35", "husky": "^1.1.2", "lerna": "^3.2.1", "lint-staged": "^8.0.4", diff --git a/packages/superset-ui-chart/package.json b/packages/superset-ui-chart/package.json index e6ba2834a..41175acb2 100644 --- a/packages/superset-ui-chart/package.json +++ b/packages/superset-ui-chart/package.json @@ -27,12 +27,15 @@ }, "dependencies": { "@superset-ui/core": "^0.7.0", + "@types/react": "^16.7.17", + "@types/react-loadable": "^5.4.2", "prop-types": "^15.6.2", "react-loadable": "^5.5.0", "reselect": "^4.0.0" }, "devDependencies": { "@superset-ui/connection": "^0.7.0", + "@types/fetch-mock": "^6.0.4", "fetch-mock": "^7.2.5", "node-fetch": "^2.2.0", "react": "^15 || ^16" diff --git a/packages/superset-ui-chart/src/clients/ChartClient.ts b/packages/superset-ui-chart/src/clients/ChartClient.ts index 70993a10f..6c2e7e0d9 100644 --- a/packages/superset-ui-chart/src/clients/ChartClient.ts +++ b/packages/superset-ui-chart/src/clients/ChartClient.ts @@ -1,18 +1,22 @@ import { isDefined } from '@superset-ui/core'; import { SupersetClient, + SupersetClientInterface, RequestConfig, - SupersetClientClass, - SupersetClientResponse, Json, + SupersetClientClass, } from '@superset-ui/connection'; import getChartBuildQueryRegistry from '../registries/ChartBuildQueryRegistrySingleton'; import { FormData, AnnotationLayerMetadata } from '../query/FormData'; -interface SliceIdAndOrFormData { - sliceId?: number; - formData?: FormData; -} +export type SliceIdAndOrFormData = + | { + sliceId: number; + formData?: Partial; + } + | { + formData: FormData; + }; interface AnnotationData { [key: string]: object; @@ -26,14 +30,11 @@ interface ChartData { } export interface ChartClientConfig { - client?: SupersetClientClass; + client?: SupersetClientInterface | SupersetClientClass; } export class ChartClient { - readonly client: { - get: (config: RequestConfig) => Promise; - post: (config: RequestConfig) => Promise; - }; + readonly client: SupersetClientInterface | SupersetClientClass; constructor(config: ChartClientConfig = {}) { const { client = SupersetClient } = config; @@ -42,7 +43,7 @@ export class ChartClient { loadFormData(input: SliceIdAndOrFormData, options?: RequestConfig): Promise { /* If sliceId is provided, use it to fetch stored formData from API */ - if (input.sliceId) { + if ('sliceId' in input) { const promise = this.client .get({ endpoint: `/api/v1/formData/?slice_id=${input.sliceId}`, @@ -55,20 +56,15 @@ export class ChartClient { * If formData is also specified, override API result * with user-specified formData */ - return input.formData - ? promise.then( - (dbFormData: object) => - ({ - ...dbFormData, - ...input.formData, - } as FormData), - ) - : promise.then((dbFormData: object) => dbFormData as FormData); + return promise.then((dbFormData: FormData) => ({ + ...dbFormData, + ...input.formData, + })); } /* If sliceId is not provided, returned formData wrapped in a Promise */ return input.formData - ? Promise.resolve(input.formData) + ? Promise.resolve(input.formData as FormData) : Promise.reject(new Error('At least one of sliceId or formData must be specified')); } diff --git a/packages/superset-ui-chart/src/components/createLoadableRenderer.js b/packages/superset-ui-chart/src/components/createLoadableRenderer.js deleted file mode 100644 index f47324aad..000000000 --- a/packages/superset-ui-chart/src/components/createLoadableRenderer.js +++ /dev/null @@ -1,47 +0,0 @@ -import Loadable from 'react-loadable'; -import PropTypes from 'prop-types'; - -const propTypes = { - onRenderFailure: PropTypes.func, - onRenderSuccess: PropTypes.func, -}; - -const defaultProps = { - onRenderFailure() {}, - onRenderSuccess() {}, -}; - -export default function createLoadableRenderer(options) { - /* eslint-disable-next-line babel/new-cap */ - const LoadableRenderer = Loadable.Map(options); - - // Extends the behavior of LoadableComponent - // generated by react-loadable - // to provide post-render listeners - class CustomLoadableRenderer extends LoadableRenderer { - componentDidMount() { - this.afterRender(); - } - - componentDidUpdate() { - this.afterRender(); - } - - afterRender() { - const { loaded, loading, error } = this.state; - if (!loading) { - if (error) { - this.props.onRenderFailure(error); - } else if (loaded && Object.keys(loaded).length > 0) { - this.props.onRenderSuccess(); - } - } - } - } - - CustomLoadableRenderer.defaultProps = defaultProps; - CustomLoadableRenderer.propTypes = propTypes; - CustomLoadableRenderer.preload = LoadableRenderer.preload; - - return CustomLoadableRenderer; -} diff --git a/packages/superset-ui-chart/src/components/createLoadableRenderer.ts b/packages/superset-ui-chart/src/components/createLoadableRenderer.ts new file mode 100644 index 000000000..bdf6c3cd0 --- /dev/null +++ b/packages/superset-ui-chart/src/components/createLoadableRenderer.ts @@ -0,0 +1,52 @@ +import Loadable from 'react-loadable'; + +export type LoadableRendererProps = { + onRenderFailure?: Function; + onRenderSuccess?: Function; +}; + +const defaultProps = { + onRenderFailure() {}, + onRenderSuccess() {}, +}; + +export interface LoadableRenderer + extends React.ComponentClass, + Loadable.LoadableComponent {} + +export default function createLoadableRenderer( + options: Loadable.OptionsWithMap, +): LoadableRenderer { + // eslint-disable-next-line babel/new-cap + const LoadableRenderer = Loadable.Map(options) as LoadableRenderer; + + // Extends the behavior of LoadableComponent to provide post-render listeners + class CustomLoadableRenderer extends LoadableRenderer { + static defaultProps: object; + + componentDidMount() { + this.afterRender(); + } + + componentDidUpdate() { + this.afterRender(); + } + + afterRender() { + const { loaded, loading, error } = this.state; + const { onRenderFailure, onRenderSuccess } = this.props; + if (!loading) { + if (error) { + (onRenderFailure as Function)(error); + } else if (loaded && Object.keys(loaded).length > 0) { + (onRenderSuccess as Function)(); + } + } + } + } + + CustomLoadableRenderer.defaultProps = defaultProps; + CustomLoadableRenderer.preload = LoadableRenderer.preload; + + return CustomLoadableRenderer; +} diff --git a/packages/superset-ui-chart/src/components/reactify.jsx b/packages/superset-ui-chart/src/components/reactify.jsx deleted file mode 100644 index a29f7954b..000000000 --- a/packages/superset-ui-chart/src/components/reactify.jsx +++ /dev/null @@ -1,51 +0,0 @@ -import React from 'react'; - -export default function reactify(renderFn) { - class ReactifiedComponent extends React.Component { - constructor(props) { - super(props); - this.setContainerRef = this.setContainerRef.bind(this); - } - - componentDidMount() { - this.execute(); - } - - componentDidUpdate() { - this.execute(); - } - - componentWillUnmount() { - this.container = null; - } - - setContainerRef(c) { - this.container = c; - } - - execute() { - if (this.container) { - renderFn(this.container, this.props); - } - } - - render() { - const { id, className } = this.props; - - return
; - } - } - - if (renderFn.displayName) { - ReactifiedComponent.displayName = renderFn.displayName; - } - /* eslint-disable-next-line react/forbid-foreign-prop-types */ - if (renderFn.propTypes) { - ReactifiedComponent.propTypes = renderFn.propTypes; - } - if (renderFn.defaultProps) { - ReactifiedComponent.defaultProps = renderFn.defaultProps; - } - - return ReactifiedComponent; -} diff --git a/packages/superset-ui-chart/src/components/reactify.tsx b/packages/superset-ui-chart/src/components/reactify.tsx new file mode 100644 index 000000000..4dc4fe990 --- /dev/null +++ b/packages/superset-ui-chart/src/components/reactify.tsx @@ -0,0 +1,82 @@ +import React from 'react'; + +// TODO: Note that id and className can collide between Props and ReactifyProps +// leading to (likely) unexpected behaviors. We should either require Props to not +// contain an id/className, or not combine them (via intersection), instead preferring +// wrapping (composition). As an example: +// interface MyProps { +// id: number; +// } +// function myRender(container: HTMLDivElement, props: Readonly): void { +// props.id // unusable: id is string & number +// } +// new (reactify(myRender))({ id: 5 }); // error: id has to be string & number + +export type ReactifyProps = { + id: string; + className?: string; +}; + +export interface RenderFuncType { + (container: HTMLDivElement, props: Readonly): void; + displayName?: string; + defaultProps?: Partial; + propTypes?: React.WeakValidationMap; +} + +export default function reactify( + renderFn: RenderFuncType, +): React.ComponentClass { + class ReactifiedComponent extends React.Component { + // eslint-disable-next-line react/sort-comp + container?: HTMLDivElement; + + constructor(props: Props & ReactifyProps) { + super(props); + this.setContainerRef = this.setContainerRef.bind(this); + } + + componentDidMount() { + this.execute(); + } + + componentDidUpdate() { + this.execute(); + } + + componentWillUnmount() { + this.container = undefined; + } + + setContainerRef(ref: HTMLDivElement) { + this.container = ref; + } + + execute() { + if (this.container) { + renderFn(this.container, this.props); + } + } + + render() { + const { id, className } = this.props; + + return
; + } + } + + const ReactifiedClass: React.ComponentClass = ReactifiedComponent; + + if (renderFn.displayName) { + ReactifiedClass.displayName = renderFn.displayName; + } + // eslint-disable-next-line react/forbid-foreign-prop-types + if (renderFn.propTypes) { + ReactifiedClass.propTypes = { ...ReactifiedClass.propTypes, ...renderFn.propTypes }; + } + if (renderFn.defaultProps) { + ReactifiedClass.defaultProps = renderFn.defaultProps; + } + + return ReactifiedComponent; +} diff --git a/packages/superset-ui-chart/src/index.js b/packages/superset-ui-chart/src/index.ts similarity index 100% rename from packages/superset-ui-chart/src/index.js rename to packages/superset-ui-chart/src/index.ts diff --git a/packages/superset-ui-chart/src/query/FormData.ts b/packages/superset-ui-chart/src/query/FormData.ts index b55c7d0f4..e6c4df7fd 100644 --- a/packages/superset-ui-chart/src/query/FormData.ts +++ b/packages/superset-ui-chart/src/query/FormData.ts @@ -1,3 +1,5 @@ +/* eslint camelcase: 0 */ +// FormData uses snake_cased keys. import { FormDataMetric, MetricKey } from './Metric'; // Type signature and utility functions for formData shared by all viz types @@ -14,18 +16,14 @@ export type AnnotationLayerMetadata = { sourceType?: string; }; -/* eslint-disable camelcase */ type BaseFormData = { datasource: string; viz_type: string; annotation_layers?: Array; } & Metrics; -/* eslint-enable camelcase */ // FormData is either sqla-based or druid-based type SqlaFormData = { - // FormData uses snake_cased keys. - // eslint-disable-next-line camelcase granularity_sqla: string; } & BaseFormData; diff --git a/packages/superset-ui-chart/test/clients/ChartClient.test.ts b/packages/superset-ui-chart/test/clients/ChartClient.test.ts index 52ee44070..1f7fe0802 100644 --- a/packages/superset-ui-chart/test/clients/ChartClient.test.ts +++ b/packages/superset-ui-chart/test/clients/ChartClient.test.ts @@ -2,10 +2,11 @@ import fetchMock from 'fetch-mock'; import { SupersetClientClass, SupersetClient } from '@superset-ui/connection'; import { ChartClient, getChartBuildQueryRegistry, buildQueryContext, FormData } from '../../src'; +import { SliceIdAndOrFormData } from '../../src/clients/ChartClient'; import { LOGIN_GLOB } from '../../../superset-ui-connection/test/fixtures/constants'; describe('ChartClient', () => { - let chartClient; + let chartClient: ChartClient; beforeAll(() => { fetchMock.get(LOGIN_GLOB, { csrf_token: '1234' }); @@ -32,59 +33,57 @@ describe('ChartClient', () => { }); describe('.loadFormData({ sliceId, formData }, options)', () => { + const sliceId = 123; it('fetches formData if given only sliceId', () => { - fetchMock.get('glob:*/api/v1/formData/?slice_id=123', { + fetchMock.get(`glob:*/api/v1/formData/?slice_id=${sliceId}`, { form_data: { granularity: 'minute', - field1: 'abc', - field2: 'def', + viz_type: 'line', }, }); - return expect(chartClient.loadFormData({ sliceId: 123 })).resolves.toEqual({ + return expect(chartClient.loadFormData({ sliceId })).resolves.toEqual({ granularity: 'minute', - field1: 'abc', - field2: 'def', + viz_type: 'line', }); }); it('fetches formData from sliceId and merges with specify formData if both fields are specified', () => { - fetchMock.get('glob:*/api/v1/formData/?slice_id=123', { + fetchMock.get(`glob:*/api/v1/formData/?slice_id=${sliceId}`, { form_data: { granularity: 'minute', - field1: 'abc', - field2: 'def', + viz_type: 'line', }, }); return expect( chartClient.loadFormData({ - sliceId: 123, + sliceId, formData: { - field2: 'ghi', - field3: 'jkl', + granularity: 'second', + viz_type: 'bar', }, }), ).resolves.toEqual({ - granularity: 'minute', - field1: 'abc', - field2: 'ghi', - field3: 'jkl', + granularity: 'second', + viz_type: 'bar', }); }); it('returns promise of formData if only formData was given', () => expect( chartClient.loadFormData({ formData: { - field1: 'abc', - field2: 'def', + datasource: '1__table', + granularity: 'minute', + viz_type: 'line', }, }), ).resolves.toEqual({ - field1: 'abc', - field2: 'def', + datasource: '1__table', + granularity: 'minute', + viz_type: 'line', })); it('rejects if none of sliceId or formData is specified', () => - expect(chartClient.loadFormData({})).rejects.toEqual( + expect(chartClient.loadFormData({} as SliceIdAndOrFormData)).rejects.toEqual( new Error('At least one of sliceId or formData must be specified'), )); }); @@ -104,8 +103,6 @@ describe('ChartClient', () => { granularity: 'minute', viz_type: 'word_cloud', datasource: '1__table', - field3: 'abc', - field4: 'def', }), ).resolves.toEqual({ field1: 'abc', @@ -118,8 +115,6 @@ describe('ChartClient', () => { granularity: 'minute', viz_type: 'rainbow_3d_pie', datasource: '1__table', - field3: 'abc', - field4: 'def', }), ).rejects.toEqual(new Error('Unknown chart type: rainbow_3d_pie'))); }); @@ -179,8 +174,9 @@ describe('ChartClient', () => { }); describe('.loadChartData({ sliceId, formData })', () => { + const sliceId = 10120; it('loadAllDataNecessaryForAChart', () => { - fetchMock.get('glob:*/api/v1/formData/?slice_id=10120', { + fetchMock.get(`glob:*/api/v1/formData/?slice_id=${sliceId}`, { form_data: { granularity: 'minute', viz_type: 'line', @@ -206,7 +202,7 @@ describe('ChartClient', () => { return expect( chartClient.loadChartData({ - sliceId: '10120', + sliceId, }), ).resolves.toEqual({ annotationData: {}, diff --git a/packages/superset-ui-chart/test/components/createLoadableRenderer.test.jsx b/packages/superset-ui-chart/test/components/createLoadableRenderer.test.tsx similarity index 82% rename from packages/superset-ui-chart/test/components/createLoadableRenderer.test.jsx rename to packages/superset-ui-chart/test/components/createLoadableRenderer.test.tsx index 64d2819a9..da0f6691a 100644 --- a/packages/superset-ui-chart/test/components/createLoadableRenderer.test.jsx +++ b/packages/superset-ui-chart/test/components/createLoadableRenderer.test.tsx @@ -1,15 +1,17 @@ import React from 'react'; import { shallow } from 'enzyme'; -import { createLoadableRenderer } from '../../src'; +import createLoadableRenderer, { + LoadableRenderer as LoadableRendererType, +} from '../../src/components/createLoadableRenderer'; describe('createLoadableRenderer', () => { function TestComponent() { return
test
; } - let loadChartSuccess; - let render; - let loading; - let LoadableRenderer; + let loadChartSuccess = jest.fn(() => Promise.resolve(TestComponent)); + let render = () => null; + let loading = () => null; + let LoadableRenderer: LoadableRendererType<{}, {}>; beforeEach(() => { loadChartSuccess = jest.fn(() => Promise.resolve(TestComponent)); @@ -19,6 +21,7 @@ describe('createLoadableRenderer', () => { return ; }); loading = jest.fn(() =>
Loading
); + LoadableRenderer = createLoadableRenderer({ loader: { Chart: loadChartSuccess, @@ -83,5 +86,15 @@ describe('createLoadableRenderer', () => { done(); }, 10); }); + + it('does not throw if loaders are empty', () => { + const NeverLoadingRenderer = createLoadableRenderer({ + loader: {}, + loading, + render, + }); + + expect(() => shallow()).not.toThrow(); + }); }); }); diff --git a/packages/superset-ui-chart/test/components/reactify.test.jsx b/packages/superset-ui-chart/test/components/reactify.test.tsx similarity index 71% rename from packages/superset-ui-chart/test/components/reactify.test.jsx rename to packages/superset-ui-chart/test/components/reactify.test.tsx index 347e66a3d..dedfb2e50 100644 --- a/packages/superset-ui-chart/test/components/reactify.test.jsx +++ b/packages/superset-ui-chart/test/components/reactify.test.tsx @@ -1,10 +1,10 @@ import PropTypes from 'prop-types'; import React from 'react'; import { mount } from 'enzyme'; -import { reactify } from '../../src'; +import reactify, { RenderFuncType } from '../../src/components/reactify'; describe('reactify(renderFn)', () => { - const renderFn = jest.fn((element, props) => { + const renderFn: RenderFuncType<{ content?: string }> = jest.fn((element, props) => { const container = element; container.innerHTML = ''; const child = document.createElement('b'); @@ -13,17 +13,19 @@ describe('reactify(renderFn)', () => { }); renderFn.displayName = 'BoldText'; + renderFn.propTypes = { content: PropTypes.string, }; + renderFn.defaultProps = { content: 'ghi', }; const TheChart = reactify(renderFn); - class TestComponent extends React.PureComponent { - constructor(props) { + class TestComponent extends React.PureComponent<{}, { content: string }, any> { + constructor(props = {}) { super(props); this.state = { content: 'abc' }; } @@ -37,17 +39,18 @@ describe('reactify(renderFn)', () => { render() { const { content } = this.state; - return ; + return ; } } it('returns a React component class', done => { const wrapper = mount(); + expect(renderFn).toHaveBeenCalledTimes(1); - expect(wrapper.html()).toEqual('
abc
'); + expect(wrapper.html()).toEqual('
abc
'); setTimeout(() => { expect(renderFn).toHaveBeenCalledTimes(2); - expect(wrapper.html()).toEqual('
def
'); + expect(wrapper.html()).toEqual('
def
'); wrapper.unmount(); done(); }, 20); @@ -64,19 +67,19 @@ describe('reactify(renderFn)', () => { describe('propTypes', () => { it('has propTypes if renderFn.propTypes is defined', () => { /* eslint-disable-next-line react/forbid-foreign-prop-types */ - expect(TheChart.propTypes).toBe(renderFn.propTypes); + expect(Object.keys(TheChart.propTypes || {})).toEqual(['id', 'className', 'content']); }); it('does not have propTypes if renderFn.propTypes is not defined', () => { const AnotherChart = reactify(() => {}); /* eslint-disable-next-line react/forbid-foreign-prop-types */ - expect(AnotherChart.propTypes).toBeUndefined(); + expect(Object.keys(AnotherChart.propTypes || {})).toEqual(['id', 'className']); }); }); describe('defaultProps', () => { it('has defaultProps if renderFn.defaultProps is defined', () => { expect(TheChart.defaultProps).toBe(renderFn.defaultProps); - const wrapper = mount(); - expect(wrapper.html()).toEqual('
ghi
'); + const wrapper = mount(); + expect(wrapper.html()).toEqual('
ghi
'); }); it('does not have defaultProps if renderFn.defaultProps is not defined', () => { const AnotherChart = reactify(() => {}); @@ -85,8 +88,8 @@ describe('reactify(renderFn)', () => { }); it('does not try to render if not mounted', () => { const anotherRenderFn = jest.fn(); - const AnotherChart = reactify(anotherRenderFn); - new AnotherChart().execute(); + const AnotherChart = reactify(anotherRenderFn) as any; // enables valid new AnotherChart() call + new AnotherChart({ id: 'test' }).execute(); expect(anotherRenderFn).not.toHaveBeenCalled(); }); }); diff --git a/packages/superset-ui-chart/test/models/ChartPlugin.test.ts b/packages/superset-ui-chart/test/models/ChartPlugin.test.ts index 53ec08769..be2f466af 100644 --- a/packages/superset-ui-chart/test/models/ChartPlugin.test.ts +++ b/packages/superset-ui-chart/test/models/ChartPlugin.test.ts @@ -6,7 +6,7 @@ import { ChartProps, BuildQueryFunction, TransformPropsFunction, -} from '../../src/index'; +} from '../../src'; describe('ChartPlugin', () => { const metadata = new ChartMetadata({ @@ -37,14 +37,6 @@ describe('ChartPlugin', () => { }); expect(plugin).toBeInstanceOf(ChartPlugin); }); - it('throws an error if metadata is not specified', () => { - expect( - () => - new ChartPlugin({ - metadata: null, - }), - ).toThrowError(Error); - }); describe('buildQuery', () => { it('defaults to identity function', () => { const plugin = new ChartPlugin({ @@ -54,22 +46,28 @@ describe('ChartPlugin', () => { expect(plugin.loadBuildQuery).toBeUndefined(); }); it('uses loadBuildQuery field if specified', () => { + expect.assertions(1); const plugin = new ChartPlugin({ metadata, Chart: FakeChart, loadBuildQuery: () => buildQuery, }); - const fn = plugin.loadBuildQuery() as BuildQueryFunction; - expect(fn(FORM_DATA).queries[0]).toEqual({ granularity: 'day' }); + if (typeof plugin.loadBuildQuery === 'function') { + const fn = plugin.loadBuildQuery() as BuildQueryFunction; + expect(fn(FORM_DATA).queries[0]).toEqual({ granularity: 'day' }); + } }); it('uses buildQuery field if specified', () => { + expect.assertions(1); const plugin = new ChartPlugin({ metadata, Chart: FakeChart, buildQuery, }); - const fn = plugin.loadBuildQuery() as BuildQueryFunction; - expect(fn(FORM_DATA).queries[0]).toEqual({ granularity: 'day' }); + if (typeof plugin.loadBuildQuery === 'function') { + const fn = plugin.loadBuildQuery() as BuildQueryFunction; + expect(fn(FORM_DATA).queries[0]).toEqual({ granularity: 'day' }); + } }); }); describe('Chart', () => { diff --git a/packages/superset-ui-connection/src/SupersetClient.ts b/packages/superset-ui-connection/src/SupersetClient.ts index 01dbea153..5a180855f 100644 --- a/packages/superset-ui-connection/src/SupersetClient.ts +++ b/packages/superset-ui-connection/src/SupersetClient.ts @@ -1,5 +1,5 @@ import { ClientConfig, SupersetClientClass } from './SupersetClientClass'; -import { RequestConfig } from './types'; +import { RequestConfig, SupersetClientResponse } from './types'; let singletonClient: SupersetClientClass | undefined; @@ -11,8 +11,19 @@ function getInstance(maybeClient: SupersetClientClass | undefined): SupersetClie return maybeClient; } -const SupersetClient = { - configure: (config: ClientConfig = {}): SupersetClientClass => { +export interface SupersetClientInterface { + configure: (config?: ClientConfig) => SupersetClientClass; + get: (request: RequestConfig) => Promise; + getInstance: (maybeClient?: SupersetClientClass) => SupersetClientClass; + init: (force?: boolean) => Promise; + isAuthenticated: () => boolean; + post: (request: RequestConfig) => Promise; + reAuthenticate: () => Promise; + reset: () => void; +} + +const SupersetClient: SupersetClientInterface = { + configure: (config?: ClientConfig): SupersetClientClass => { singletonClient = new SupersetClientClass(config); return singletonClient; diff --git a/packages/superset-ui-connection/src/callApi/parseResponse.ts b/packages/superset-ui-connection/src/callApi/parseResponse.ts index 9cd81ee96..209e8efff 100644 --- a/packages/superset-ui-connection/src/callApi/parseResponse.ts +++ b/packages/superset-ui-connection/src/callApi/parseResponse.ts @@ -1,9 +1,9 @@ import { ParseMethod, SupersetClientResponse } from '../types'; function rejectIfNotOkay(response: Response): Promise { - if (!response.ok) return Promise.reject(response); + if (!response.ok) return Promise.reject(response); - return Promise.resolve(response); + return Promise.resolve(response); } export default function parseResponse( @@ -13,7 +13,7 @@ export default function parseResponse( const checkedPromise = apiPromise.then(rejectIfNotOkay); if (parseMethod === null) { - return checkedPromise; + return apiPromise.then(rejectIfNotOkay); } else if (parseMethod === 'text') { return checkedPromise.then(response => response.text().then(text => ({ response, text }))); } else if (parseMethod === 'json') { diff --git a/packages/superset-ui-connection/src/index.ts b/packages/superset-ui-connection/src/index.ts index 04f003d37..f2db59fa8 100644 --- a/packages/superset-ui-connection/src/index.ts +++ b/packages/superset-ui-connection/src/index.ts @@ -1,4 +1,4 @@ export { default as callApi } from './callApi'; -export { default as SupersetClient } from './SupersetClient'; +export { default as SupersetClient, SupersetClientInterface } from './SupersetClient'; export { SupersetClientClass } from './SupersetClientClass'; export * from './types'; diff --git a/packages/superset-ui-connection/src/types.ts b/packages/superset-ui-connection/src/types.ts index 135a6d7b6..19c1e82b5 100644 --- a/packages/superset-ui-connection/src/types.ts +++ b/packages/superset-ui-connection/src/types.ts @@ -10,7 +10,7 @@ export type PostPayload = { [key: string]: any }; export type Mode = RequestInit['mode']; export type Redirect = RequestInit['redirect']; export type ClientTimeout = number | undefined; -export type ParseMethod = 'json' | 'text'; +export type ParseMethod = 'json' | 'text' | null; export type Signal = RequestInit['signal']; export type Stringify = boolean; export type Url = string; diff --git a/packages/superset-ui-connection/test/SupersetClientClass.test.ts b/packages/superset-ui-connection/test/SupersetClientClass.test.ts index 67ae3b1b8..92f0d0336 100644 --- a/packages/superset-ui-connection/test/SupersetClientClass.test.ts +++ b/packages/superset-ui-connection/test/SupersetClientClass.test.ts @@ -17,7 +17,8 @@ describe('SupersetClientClass', () => { }); describe('.getUrl()', () => { - let client; + let client = new SupersetClientClass(); + beforeEach(() => { client = new SupersetClientClass({ protocol: 'https:', host: 'CONFIG_HOST' }); }); @@ -155,7 +156,7 @@ describe('SupersetClientClass', () => { it('returns true if a token is passed at configuration', () => { expect.assertions(2); - const clientWithoutToken = new SupersetClientClass({ csrfToken: null }); + const clientWithoutToken = new SupersetClientClass({ csrfToken: undefined }); const clientWithToken = new SupersetClientClass({ csrfToken: 'token' }); expect(clientWithoutToken.isAuthenticated()).toBe(false); @@ -288,7 +289,9 @@ describe('SupersetClientClass', () => { const fetchRequest = fetchMock.calls(mockGetUrl)[0][1]; expect(fetchRequest.mode).toBe(clientConfig.mode); expect(fetchRequest.credentials).toBe(clientConfig.credentials); - expect(fetchRequest.headers).toEqual(expect.objectContaining(clientConfig.headers)); + expect(fetchRequest.headers).toEqual( + expect.objectContaining(clientConfig.headers as Object), + ); return Promise.resolve(); }), @@ -361,7 +364,7 @@ describe('SupersetClientClass', () => { expect(fetchRequest.mode).toBe(overrideConfig.mode); expect(fetchRequest.credentials).toBe(overrideConfig.credentials); expect(fetchRequest.headers).toEqual( - expect.objectContaining(overrideConfig.headers), + expect.objectContaining(overrideConfig.headers as Object), ); return Promise.resolve(); @@ -412,7 +415,9 @@ describe('SupersetClientClass', () => { const fetchRequest = fetchMock.calls(mockPostUrl)[0][1]; expect(fetchRequest.mode).toBe(overrideConfig.mode); expect(fetchRequest.credentials).toBe(overrideConfig.credentials); - expect(fetchRequest.headers).toEqual(expect.objectContaining(overrideConfig.headers)); + expect(fetchRequest.headers).toEqual( + expect.objectContaining(overrideConfig.headers as Object), + ); return Promise.resolve(); }), @@ -436,7 +441,7 @@ describe('SupersetClientClass', () => { it('passes postPayload key,values in the body', () => { expect.assertions(3); - const postPayload = { number: 123, array: [1, 2, 3] }; + const postPayload = { number: 123, array: [1, 2, 3] } as any; const client = new SupersetClientClass({ protocol, host }); return client.init().then(() => @@ -454,7 +459,7 @@ describe('SupersetClientClass', () => { it('respects the stringify parameter for postPayload key,values', () => { expect.assertions(3); - const postPayload = { number: 123, array: [1, 2, 3] }; + const postPayload = { number: 123, array: [1, 2, 3] } as any; const client = new SupersetClientClass({ protocol, host }); return client.init().then(() => diff --git a/packages/superset-ui-connection/test/callApi/callApi.test.ts b/packages/superset-ui-connection/test/callApi/callApi.test.ts index 6a3795e67..7813843e3 100644 --- a/packages/superset-ui-connection/test/callApi/callApi.test.ts +++ b/packages/superset-ui-connection/test/callApi/callApi.test.ts @@ -62,7 +62,7 @@ describe('callApi()', () => { expect(fetchParams.mode).toBe(mockRequest.mode); expect(fetchParams.cache).toBe(mockRequest.cache); expect(fetchParams.credentials).toBe(mockRequest.credentials); - expect(fetchParams.headers).toEqual(expect.objectContaining(mockRequest.headers)); + expect(fetchParams.headers).toEqual(expect.objectContaining(mockRequest.headers as Object)); expect(fetchParams.redirect).toBe(mockRequest.redirect); expect(fetchParams.signal).toBe(mockRequest.signal); expect(fetchParams.body).toBe(mockRequest.body); @@ -75,7 +75,7 @@ describe('callApi()', () => { describe('POST requests', () => { it('encodes key,value pairs from postPayload', () => { expect.assertions(3); - const postPayload = { key: 'value', anotherKey: 1237 }; + const postPayload = { key: 'value', anotherKey: 1237 } as any; return callApi({ url: mockPostUrl, method: 'POST', postPayload }).then(() => { const calls = fetchMock.calls(mockPostUrl); @@ -118,7 +118,7 @@ describe('callApi()', () => { object: { a: 'a', 1: 1 }, null: null, emptyString: '', - }; + } as any; expect.assertions(1 + 2 * Object.keys(postPayload).length); diff --git a/packages/superset-ui-connection/test/callApi/parseResponse.test.ts b/packages/superset-ui-connection/test/callApi/parseResponse.test.ts index bae3ca32f..8d789f74c 100644 --- a/packages/superset-ui-connection/test/callApi/parseResponse.test.ts +++ b/packages/superset-ui-connection/test/callApi/parseResponse.test.ts @@ -4,6 +4,7 @@ import parseResponse from '../../src/callApi/parseResponse'; import { LOGIN_GLOB } from '../fixtures/constants'; import throwIfCalled from '../utils/throwIfCalled'; +import { SupersetClientResponse } from '../../src'; describe('parseResponse()', () => { beforeAll(() => { @@ -94,7 +95,8 @@ describe('parseResponse()', () => { const apiPromise = callApi({ url: mockNoParseUrl, method: 'GET' }); - return parseResponse(apiPromise, null).then((response: Response) => { + return parseResponse(apiPromise, null).then((clientResponse: SupersetClientResponse) => { + const response = clientResponse as Response; expect(fetchMock.calls(mockNoParseUrl)).toHaveLength(1); expect(response.bodyUsed).toBe(false); diff --git a/packages/superset-ui-connection/test/utils/throwIfCalled.ts b/packages/superset-ui-connection/test/utils/throwIfCalled.ts index e3b4f694c..735a345a3 100644 --- a/packages/superset-ui-connection/test/utils/throwIfCalled.ts +++ b/packages/superset-ui-connection/test/utils/throwIfCalled.ts @@ -1,3 +1,3 @@ -export default function throwIfCalled(args) { +export default function throwIfCalled(args: any) { throw new Error(`Unexpected call to throwIfCalled(): ${JSON.stringify(args)}`); } diff --git a/packages/superset-ui-connection/types/external.d.ts b/packages/superset-ui-connection/types/external.d.ts new file mode 100644 index 000000000..9a8fd4a70 --- /dev/null +++ b/packages/superset-ui-connection/types/external.d.ts @@ -0,0 +1 @@ +declare module 'fetch-mock'; diff --git a/packages/superset-ui-time-format/test/factories/createD3TimeFormatter.test.js b/packages/superset-ui-time-format/test/factories/createD3TimeFormatter.test.js index 6e47b0c98..4b8fedf13 100644 --- a/packages/superset-ui-time-format/test/factories/createD3TimeFormatter.test.js +++ b/packages/superset-ui-time-format/test/factories/createD3TimeFormatter.test.js @@ -5,6 +5,7 @@ import { TimeFormats } from '../../src'; describe('createD3TimeFormatter(config)', () => { it('requires config.formatString', () => { expect(() => createD3TimeFormatter()).toThrow(); + expect(() => createD3TimeFormatter({})).toThrow(); }); describe('if config.useLocalTime is true', () => { it('formats in local time', () => {