Skip to content

Commit

Permalink
[build] fix typescript builds (apache-superset#56)
Browse files Browse the repository at this point in the history
* [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
  • Loading branch information
williaster authored Dec 14, 2018
1 parent 9b0eec2 commit 4e9ea81
Show file tree
Hide file tree
Showing 23 changed files with 273 additions and 209 deletions.
13 changes: 7 additions & 6 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand Down
3 changes: 3 additions & 0 deletions packages/superset-ui-chart/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
40 changes: 18 additions & 22 deletions packages/superset-ui-chart/src/clients/ChartClient.ts
Original file line number Diff line number Diff line change
@@ -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: FormData;
};

interface AnnotationData {
[key: string]: object;
Expand All @@ -26,14 +30,11 @@ interface ChartData {
}

export interface ChartClientConfig {
client?: SupersetClientClass;
client?: SupersetClientInterface | SupersetClientClass;
}

export class ChartClient {
readonly client: {
get: (config: RequestConfig) => Promise<SupersetClientResponse>;
post: (config: RequestConfig) => Promise<SupersetClientResponse>;
};
readonly client: SupersetClientInterface | SupersetClientClass;

constructor(config: ChartClientConfig = {}) {
const { client = SupersetClient } = config;
Expand All @@ -42,7 +43,7 @@ export class ChartClient {

loadFormData(input: SliceIdAndOrFormData, options?: RequestConfig): Promise<FormData> {
/* 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}`,
Expand All @@ -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'));
}

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import Loadable from 'react-loadable';

export type LoadableRendererProps = {
onRenderFailure?: Function;
onRenderSuccess?: Function;
};

const defaultProps = {
onRenderFailure() {},
onRenderSuccess() {},
};

export interface LoadableRenderer<Props, Exports>
extends React.ComponentClass<Props & LoadableRendererProps>,
Loadable.LoadableComponent {}

export default function createLoadableRenderer<Props, Exports>(
options: Loadable.OptionsWithMap<Props, Exports>,
): LoadableRenderer<Props, Exports> {
// eslint-disable-next-line babel/new-cap
const LoadableRenderer = Loadable.Map(options) as LoadableRenderer<Props, Exports>;

// 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;
}
51 changes: 0 additions & 51 deletions packages/superset-ui-chart/src/components/reactify.jsx

This file was deleted.

82 changes: 82 additions & 0 deletions packages/superset-ui-chart/src/components/reactify.tsx
Original file line number Diff line number Diff line change
@@ -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<MyProps>): 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<Props> {
(container: HTMLDivElement, props: Readonly<Props & ReactifyProps>): void;
displayName?: string;
defaultProps?: Partial<Props & ReactifyProps>;
propTypes?: React.WeakValidationMap<Props & ReactifyProps>;
}

export default function reactify<Props extends object>(
renderFn: RenderFuncType<Props>,
): React.ComponentClass<Props & ReactifyProps> {
class ReactifiedComponent extends React.Component<Props & ReactifyProps> {
// 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 <div id={id} className={className} ref={this.setContainerRef} />;
}
}

const ReactifiedClass: React.ComponentClass<Props & ReactifyProps> = 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;
}
File renamed without changes.
6 changes: 2 additions & 4 deletions packages/superset-ui-chart/src/query/FormData.ts
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -14,18 +16,14 @@ export type AnnotationLayerMetadata = {
sourceType?: string;
};

/* eslint-disable camelcase */
type BaseFormData = {
datasource: string;
viz_type: string;
annotation_layers?: Array<AnnotationLayerMetadata>;
} & 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;

Expand Down
Loading

0 comments on commit 4e9ea81

Please sign in to comment.