Skip to content

Commit

Permalink
[SIP-9] Introduce TypeScript (#6120)
Browse files Browse the repository at this point in the history
* [SIP-9] Introduce TypeScript

- Introduce TypeScript and co to both source and tests
- Define alias for src directory in both webpack config and jest config so we can avoid using long relative paths like ../../src in both source and tests
- Type check feature flags system to prevent typos of flag names
- Change the feature flags system and the flags on window instead of populating them through the state tree. When introducing the first SCOPED_FILTER feature flag, it became too difficult to pipe the flags through the state initializers and layers of components and containers (the resulting code is hard to read and has a handful of methods taking an additional feature flag map parameter). Given that feature flags don't change throughout the life time of the app, it is better to leave them on window for easy access than piping them through the global state tree, which is meant to store the state of the app which changes frequently.
- Add a barebone filter panel that only shows when the SCOPED_FILTER feature flag is on

* Remove unnecessary dev-dependency on gl

* - Adding linting for TypeScript files via tslint.
- Fixing linting for Javascript files importing Typscript files
- Also fix linting for Javascript files that now leverage the webpack alias for the src directory

- up Typescript and type def versions

* Rename src directory's webpack alias from @ to src to be more explicit.
  • Loading branch information
xtinec authored and mistercrunch committed Oct 24, 2018
1 parent 9e6b171 commit 5f1eaa4
Show file tree
Hide file tree
Showing 31 changed files with 1,120 additions and 1,122 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
*.pyc
*.sqllite
*.swp
.cache-loader
.coverage
.DS_Store
.eggs
Expand Down
15 changes: 15 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,21 @@ npm run dev-server -- --supersetPort=8081

After adding or upgrading an NPM package by changing `package.json`, you must run `yarn install`, which will regenerate the `yarn.lock` file. Then, be sure to commit the new `yarn.lock` so that other users' builds are reproducible. See [the Yarn docs](https://yarnpkg.com/blog/2016/11/24/lockfiles-for-all/) for more information.

#### Feature flags

Superset supports a server-wide feature flag system, which eases the incremental development of features. To add a new feature flag, simply modify `superset_config.py` with something like the following:
```
FEATURE_FLAGS = {
'SCOPED_FILTER': True,
}
```
If you want to use the same flag in the client code, also add it to the FeatureFlag TypeScript enum in `superset/assets/src/featureFlags.ts`. For example,
```
export enum FeatureFlag {
SCOPED_FILTER = 'SCOPED_FILTER',
}
```

## Testing

All tests are carried out in [tox](http://tox.readthedocs.io/en/latest/index.html)
Expand Down
7 changes: 5 additions & 2 deletions superset/assets/.eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
"func-names": 0,
"react/jsx-no-bind": 0,
"no-confusing-arrow": 0,

"jsx-a11y/no-static-element-interactions": 0,
"jsx-a11y/anchor-has-content": 0,
"react/require-default-props": 0,
Expand All @@ -40,6 +39,10 @@
"react/no-string-refs": 0,
"indent": 0,
"no-multi-spaces": 0,
"padded-blocks": 0
"padded-blocks": 0,
"import/extensions": [".js", ".jsx", ".ts", ".tsx", ".json"]
},
"settings": {
"import/resolver": "webpack"
}
}
10 changes: 8 additions & 2 deletions superset/assets/jest.config.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,17 @@
module.exports = {
testRegex: '\\/spec\\/.*_spec\\.jsx?$',
testRegex: '\\/spec\\/.*_spec\\.(j|t)sx?$',
moduleNameMapper: {
'\\.(css|less)$': '<rootDir>/spec/__mocks__/styleMock.js',
'\\.(gif|ttf|eot|svg)$': '<rootDir>/spec/__mocks__/fileMock.js',
'^src/(.*)$': '<rootDir>/src/$1',
},
setupTestFrameworkScriptFile: '<rootDir>/spec/helpers/shim.js',
testURL: 'http://localhost',
collectCoverageFrom: ['src/**/*.{js,jsx}'],
collectCoverageFrom: ['src/**/*.{js,jsx,ts,tsx}'],
coverageDirectory: '<rootDir>/coverage/',
transform: {
'^.+\\.jsx?$': 'babel-jest',
'^.+\\.tsx?$': 'ts-jest',
},
moduleFileExtensions: ['ts', 'tsx', 'js', 'jsx', 'json'],
};
17 changes: 14 additions & 3 deletions superset/assets/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
"dev-server": "webpack-dev-server --mode=development --progress",
"prod": "node --max_old_space_size=4096 webpack --mode=production --colors --progress",
"build": "NODE_ENV=production webpack --mode=production --colors --progress",
"lint": "eslint --ignore-path=.eslintignore --ext .js,.jsx .",
"lint-fix": "eslint --fix --ignore-path=.eslintignore --ext .js,.jsx .",
"lint": "eslint --ignore-path=.eslintignore --ext .js,.jsx . && tslint -c tslint.json ./src/**/*.ts{,x}",
"lint-fix": "eslint --fix --ignore-path=.eslintignore --ext .js,.jsx . && tslint -c tslint.json --fix ./src/**/*.ts{,x}",
"sync-backend": "babel-node --presets env src/syncBackend.js",
"cypress": "cypress",
"cypress-debug": "cypress open --config watchForFileChanges=true"
Expand Down Expand Up @@ -131,6 +131,9 @@
"viewport-mercator-project": "^5.0.0"
},
"devDependencies": {
"@types/jest": "^23.3.5",
"@types/react": "^16.4.18",
"@types/react-dom": "^16.0.9",
"babel-cli": "^6.14.0",
"babel-core": "^6.10.4",
"babel-eslint": "^8.2.2",
Expand All @@ -143,6 +146,7 @@
"babel-polyfill": "^6.23.0",
"babel-preset-airbnb": "^2.1.1",
"babel-preset-env": "^1.7.0",
"cache-loader": "^1.2.2",
"clean-webpack-plugin": "^0.1.19",
"css-loader": "^1.0.0",
"cypress": "^3.0.3",
Expand All @@ -151,6 +155,7 @@
"eslint": "^4.19.0",
"eslint-config-airbnb": "^15.0.1",
"eslint-config-prettier": "^2.9.0",
"eslint-import-resolver-webpack": "^0.10.1",
"eslint-plugin-cypress": "^2.0.1",
"eslint-plugin-import": "^2.2.0",
"eslint-plugin-jest": "^21.24.1",
Expand All @@ -161,7 +166,7 @@
"exports-loader": "^0.7.0",
"fetch-mock": "^7.0.0-alpha.6",
"file-loader": "^1.1.11",
"gl": "^4.0.4",
"fork-ts-checker-webpack-plugin": "^0.4.9",
"ignore-styles": "^5.0.1",
"imports-loader": "^0.7.1",
"jest": "^23.6.0",
Expand All @@ -181,7 +186,13 @@
"speed-measure-webpack-plugin": "^1.2.3",
"style-loader": "^0.21.0",
"terser-webpack-plugin": "^1.1.0",
"thread-loader": "^1.2.0",
"transform-loader": "^0.2.3",
"ts-jest": "^23.10.4",
"ts-loader": "^5.2.0",
"tslint": "^5.11.0",
"tslint-react": "^3.6.0",
"typescript": "^3.1.3",
"url-loader": "^1.0.1",
"webpack": "^4.19.0",
"webpack-assets-manifest": "^3.0.1",
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
import React from 'react';
import { shallow } from 'enzyme';
import { getFormDataFromControls, defaultControls }
from '../../../../src/explore/store';
import {
ControlPanelsContainer,
} from '../../../../src/explore/components/ControlPanelsContainer';
import ControlPanelSection from '../../../../src/explore/components/ControlPanelSection';
import { getFormDataFromControls, defaultControls } from 'src/explore/store';
import { ControlPanelsContainer } from 'src/explore/components/ControlPanelsContainer';
import ControlPanelSection from 'src/explore/components/ControlPanelSection';
import * as featureFlags from 'src/featureFlags';

const defaultProps = {
datasource_type: 'table',
Expand All @@ -18,12 +16,22 @@ const defaultProps = {

describe('ControlPanelsContainer', () => {
let wrapper;
let scopedFilterOn = false;
const isFeatureEnabledMock = jest.spyOn(featureFlags, 'isFeatureEnabled')
.mockImplementation(() => scopedFilterOn);

beforeEach(() => {
wrapper = shallow(<ControlPanelsContainer {...defaultProps} />);
afterAll(() => {
isFeatureEnabledMock.mockRestore();
});

it('renders ControlPanelSections', () => {
wrapper = shallow(<ControlPanelsContainer {...defaultProps} />);
expect(wrapper.find(ControlPanelSection)).toHaveLength(6);
});

it('renders filter panel when SCOPED_FILTER flag is on', () => {
scopedFilterOn = true;
wrapper = shallow(<ControlPanelsContainer {...defaultProps} />);
expect(wrapper.find(ControlPanelSection)).toHaveLength(7);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -3,51 +3,46 @@ import configureStore from 'redux-mock-store';
import thunk from 'redux-thunk';
import { shallow } from 'enzyme';

import getInitialState from '../../../../src/explore/reducers/getInitialState';
import ExploreViewContainer
from '../../../../src/explore/components/ExploreViewContainer';
import QueryAndSaveBtns
from '../../../../src/explore/components/QueryAndSaveBtns';
import ControlPanelsContainer
from '../../../../src/explore/components/ControlPanelsContainer';
import ChartContainer
from '../../../../src/explore/components/ExploreChartPanel';
import getInitialState from 'src/explore/reducers/getInitialState';
import ExploreViewContainer from 'src/explore/components/ExploreViewContainer';
import QueryAndSaveBtns from 'src/explore/components/QueryAndSaveBtns';
import ControlPanelsContainer from 'src/explore/components/ControlPanelsContainer';
import ChartContainer from 'src/explore/components/ExploreChartPanel';
import * as featureFlags from 'src/featureFlags';

describe('ExploreViewContainer', () => {
const middlewares = [thunk];
const mockStore = configureStore(middlewares);
let store;
let wrapper;
let isFeatureEnabledMock;

beforeAll(() => {
isFeatureEnabledMock = jest.spyOn(featureFlags, 'isFeatureEnabled')
.mockReturnValue(false);

const bootstrapData = {
common: {
feature_flags: {
FOO_BAR: true,
},
conf: {},
},
datasource: {
columns: [],
},
form_data: {
datasource: {},
},
};
store = mockStore(getInitialState(bootstrapData), {});
});

afterAll(() => {
isFeatureEnabledMock.mockRestore();
});

beforeEach(() => {
wrapper = shallow(<ExploreViewContainer />, {
context: { store },
disableLifecycleMethods: true,
});
});

it('should set feature flags', () => {
expect(wrapper.prop('isFeatureEnabled')('FOO_BAR')).toBe(true);
});

it('renders', () => {
expect(
React.isValidElement(<ExploreViewContainer />),
Expand Down
23 changes: 4 additions & 19 deletions superset/assets/spec/javascripts/sqllab/App_spec.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,35 +5,20 @@ import thunk from 'redux-thunk';
import { shallow } from 'enzyme';
import sinon from 'sinon';

import App from '../../../src/SqlLab/components/App';
import TabbedSqlEditors from '../../../src/SqlLab/components/TabbedSqlEditors';
import getInitialState from '../../../src/SqlLab/getInitialState';
import App from 'src/SqlLab/components/App';
import TabbedSqlEditors from 'src/SqlLab/components/TabbedSqlEditors';
import { sqlLabReducer } from 'src/SqlLab/reducers';

describe('SqlLab App', () => {
const middlewares = [thunk];
const mockStore = configureStore(middlewares);
let store;
const store = mockStore(sqlLabReducer(undefined, {}), {});
let wrapper;

beforeAll(() => {
const bootstrapData = {
common: {
feature_flags: {
FOO_BAR: true,
},
},
};
store = mockStore(getInitialState(bootstrapData), {});
});

beforeEach(() => {
wrapper = shallow(<App />, { context: { store } });
});

it('should set feature flags', () => {
expect(wrapper.prop('isFeatureEnabled')('FOO_BAR')).toBe(true);
});

it('is valid', () => {
expect(React.isValidElement(<App />)).toBe(true);
});
Expand Down
2 changes: 2 additions & 0 deletions superset/assets/src/SqlLab/App.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { Provider } from 'react-redux';
import thunkMiddleware from 'redux-thunk';
import { hot } from 'react-hot-loader';

import { initFeatureFlags } from 'src/featureFlags';
import getInitialState from './getInitialState';
import rootReducer from './reducers';
import { initEnhancer } from '../reduxUtils';
Expand All @@ -18,6 +19,7 @@ appSetup();

const appContainer = document.getElementById('app');
const bootstrapData = JSON.parse(appContainer.getAttribute('data-bootstrap'));
initFeatureFlags(bootstrapData.common.feature_flags);
const state = getInitialState(bootstrapData);

const store = createStore(
Expand Down
7 changes: 1 addition & 6 deletions superset/assets/src/SqlLab/components/App.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import QueryAutoRefresh from './QueryAutoRefresh';
import QuerySearch from './QuerySearch';
import ToastPresenter from '../../messageToasts/containers/ToastPresenter';
import * as Actions from '../actions';
import { isFeatureEnabledCreator } from '../../featureFlags';

class App extends React.PureComponent {
constructor(props) {
Expand Down Expand Up @@ -84,10 +83,6 @@ App.propTypes = {
actions: PropTypes.object,
};

const mapStateToProps = state => ({
isFeatureEnabled: isFeatureEnabledCreator(state),
});

function mapDispatchToProps(dispatch) {
return {
actions: bindActionCreators(Actions, dispatch),
Expand All @@ -96,6 +91,6 @@ function mapDispatchToProps(dispatch) {

export { App };
export default connect(
mapStateToProps,
null,
mapDispatchToProps,
)(App);
1 change: 0 additions & 1 deletion superset/assets/src/SqlLab/getInitialState.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ export default function getInitialState({ defaultDbId, ...restBootstrapData }) {
};

return {
featureFlags: restBootstrapData.common.feature_flags,
sqlLab: {
activeSouthPaneTab: 'Results',
alerts: [],
Expand Down
2 changes: 0 additions & 2 deletions superset/assets/src/SqlLab/reducers.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import {
getFromArr,
addToArr,
} from '../reduxUtils';
import featureFlags from '../featureFlags';
import { t } from '../locales';

export const sqlLabReducer = function (state = {}, action) {
Expand Down Expand Up @@ -280,7 +279,6 @@ export const sqlLabReducer = function (state = {}, action) {
};

export default combineReducers({
featureFlags,
sqlLab: sqlLabReducer,
messageToasts,
});
2 changes: 2 additions & 0 deletions superset/assets/src/dashboard/App.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { createStore, applyMiddleware, compose } from 'redux';
import { Provider } from 'react-redux';
import { hot } from 'react-hot-loader';

import { initFeatureFlags } from 'src/featureFlags';
import { initEnhancer } from '../reduxUtils';
import { appSetup } from '../common';
import DashboardContainer from './containers/Dashboard';
Expand All @@ -14,6 +15,7 @@ appSetup();

const appContainer = document.getElementById('app');
const bootstrapData = JSON.parse(appContainer.getAttribute('data-bootstrap'));
initFeatureFlags(bootstrapData.common.feature_flags);
const initState = getInitialState(bootstrapData);

const store = createStore(
Expand Down
Loading

0 comments on commit 5f1eaa4

Please sign in to comment.