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

ESLint: Re-enable rule default-props-match-prop-types #10868

Merged
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
2 changes: 0 additions & 2 deletions superset-frontend/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,6 @@ module.exports = {
'padded-blocks': 0,
'prefer-arrow-callback': 0,
'prefer-destructuring': ['error', { object: true, array: false }],
'react/default-props-match-prop-types': 0, // disabled temporarily
'react/destructuring-assignment': 0, // re-enable up for discussion
'react/forbid-prop-types': 0,
'react/jsx-filename-extension': [1, { extensions: ['.jsx', '.tsx'] }],
Expand Down Expand Up @@ -231,7 +230,6 @@ module.exports = {
'prefer-arrow-callback': 0,
'prefer-object-spread': 1,
'prefer-destructuring': ['error', { object: true, array: false }],
'react/default-props-match-prop-types': 0, // disabled temporarily
'react/destructuring-assignment': 0, // re-enable up for discussion
'react/forbid-prop-types': 0,
'react/jsx-filename-extension': [1, { extensions: ['.jsx', '.tsx'] }],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,6 @@ describe('DashboardBuilder', () => {
dashboardLayout,
deleteTopLevelTabs() {},
editMode: false,
showBuilderPane() {},
setColorSchemeAndUnsavedChanges() {},
colorScheme: undefined,
handleComponentDrop() {},
setDirectPathToChild: sinon.spy(),
};
Expand Down
3 changes: 1 addition & 2 deletions superset-frontend/src/CRUD/Field.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,9 @@ const propTypes = {
compact: PropTypes.bool,
};
const defaultProps = {
controlProps: {},
onChange: () => {},
compact: false,
desc: null,
description: null,
};

export default class Field extends React.PureComponent {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,6 @@ const propTypes = {
templateParams: PropTypes.string,
};

const defaultProps = {
vizRequest: {},
};

class ExploreCtasResultsButton extends React.PureComponent {
constructor(props) {
super(props);
Expand Down Expand Up @@ -110,7 +106,6 @@ class ExploreCtasResultsButton extends React.PureComponent {
}
}
ExploreCtasResultsButton.propTypes = propTypes;
ExploreCtasResultsButton.defaultProps = defaultProps;

function mapStateToProps({ sqlLab, common }) {
return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import TableSelector from '../../components/TableSelector';

const propTypes = {
queryEditor: PropTypes.object.isRequired,
height: PropTypes.number.isRequired,
height: PropTypes.number,
tables: PropTypes.array,
actions: PropTypes.object,
database: PropTypes.object,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@ const propTypes = {
};

const defaultProps = {
label: null,
description: null,
onChange: () => {},
code: '{}',
};
Expand Down
5 changes: 0 additions & 5 deletions superset-frontend/src/components/Hotkeys.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,6 @@ const propTypes = {
placement: PropTypes.string,
};

const defaultProps = {
hotkeys: [],
};

export default class Hotkeys extends React.PureComponent {
componentDidMount() {
this.props.hotkeys.forEach(keyConfig => {
Expand Down Expand Up @@ -84,4 +80,3 @@ export default class Hotkeys extends React.PureComponent {
}

Hotkeys.propTypes = propTypes;
Hotkeys.defaultProps = defaultProps;
6 changes: 3 additions & 3 deletions superset-frontend/src/components/Select/OnPasteSelect.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,12 @@ export default class OnPasteSelect extends React.Component {
}

OnPasteSelect.propTypes = {
separator: PropTypes.array.isRequired,
separator: PropTypes.array,
selectWrap: PropTypes.elementType,
selectRef: PropTypes.func,
onChange: PropTypes.func.isRequired,
valueKey: PropTypes.string.isRequired,
labelKey: PropTypes.string.isRequired,
valueKey: PropTypes.string,
labelKey: PropTypes.string,
options: PropTypes.array,
isMulti: PropTypes.bool,
value: PropTypes.any,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@ import { getCategoricalSchemeRegistry, t } from '@superset-ui/core';
import ColorSchemeControl from 'src/explore/components/controls/ColorSchemeControl';

const propTypes = {
onChange: PropTypes.func.isRequired,
onChange: PropTypes.func,
colorScheme: PropTypes.string,
};

const defaultProps = {
colorScheme: undefined,
onChange: () => {},
Copy link
Member

Choose a reason for hiding this comment

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

this one is making me nervous, can we remove the .isRequired instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

👍

colorScheme: undefined,
};

class ColorSchemeControlWrapper extends React.PureComponent {
Expand Down
17 changes: 1 addition & 16 deletions superset-frontend/src/dashboard/components/DashboardBuilder.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,19 +54,14 @@ const propTypes = {
dashboardLayout: PropTypes.object.isRequired,
deleteTopLevelTabs: PropTypes.func.isRequired,
editMode: PropTypes.bool.isRequired,
showBuilderPane: PropTypes.func.isRequired,
colorScheme: PropTypes.string,
setColorSchemeAndUnsavedChanges: PropTypes.func.isRequired,
handleComponentDrop: PropTypes.func.isRequired,
directPathToChild: PropTypes.arrayOf(PropTypes.string),
setDirectPathToChild: PropTypes.func.isRequired,
setMountedTab: PropTypes.func.isRequired,
};

const defaultProps = {
showBuilderPane: false,
Copy link
Member

Choose a reason for hiding this comment

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

seems less risky to remove the .isRequired here too (unless you can confirm you checked all the calls)

Copy link
Member Author

Choose a reason for hiding this comment

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

I did, but you're right, let's keep the default and remove isRequired

Copy link
Member

Choose a reason for hiding this comment

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

I was confused about the type here and tracked it into the child component, and it looks unused there. Can you double check and remove all references to it here if that's the case?

Copy link
Member Author

Choose a reason for hiding this comment

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

It seems that not only showBuilderPane was unused, but also colorScheme and setColorSchemeAndUnsavedChanges. I removed references from this file, containers/DashboardBuilder and the spec file.

directPathToChild: [],
colorScheme: undefined,
};

class DashboardBuilder extends React.Component {
Expand Down Expand Up @@ -155,14 +150,7 @@ class DashboardBuilder extends React.Component {
}

render() {
const {
handleComponentDrop,
dashboardLayout,
editMode,
showBuilderPane,
setColorSchemeAndUnsavedChanges,
colorScheme,
} = this.props;
const { handleComponentDrop, dashboardLayout, editMode } = this.props;
const { tabIndex } = this.state;
const dashboardRoot = dashboardLayout[DASHBOARD_ROOT_ID];
const rootChildId = dashboardRoot.children[0];
Expand Down Expand Up @@ -272,9 +260,6 @@ class DashboardBuilder extends React.Component {
{editMode && (
<BuilderComponentPane
topOffset={HEADER_HEIGHT + (topLevelTabs ? TABS_HEIGHT : 0)}
showBuilderPane={showBuilderPane}
setColorSchemeAndUnsavedChanges={setColorSchemeAndUnsavedChanges}
colorScheme={colorScheme}
/>
)}
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import '../stylesheets/buttons.less';

const propTypes = {
dashboardId: PropTypes.number.isRequired,
show: PropTypes.bool.isRequired,
show: PropTypes.bool,
onHide: PropTypes.func,
colorScheme: PropTypes.object,
setColorSchemeAndUnsavedChanges: PropTypes.func,
Expand Down
2 changes: 1 addition & 1 deletion superset-frontend/src/dashboard/components/SliceAdder.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ const propTypes = {
lastUpdated: PropTypes.number.isRequired,
errorMessage: PropTypes.string,
userId: PropTypes.string.isRequired,
selectedSliceIds: PropTypes.arrayOf(PropTypes.number).isRequired,
selectedSliceIds: PropTypes.arrayOf(PropTypes.number),
editMode: PropTypes.bool,
height: PropTypes.number,
};
Expand Down
1 change: 0 additions & 1 deletion superset-frontend/src/dashboard/components/SliceHeader.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ const propTypes = {
const defaultProps = {
innerRef: null,
forceRefresh: () => ({}),
removeSlice: () => ({}),
updateSliceName: () => ({}),
toggleExpandSlice: () => ({}),
exploreChart: () => ({}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,6 @@ const propTypes = {
updateComponents: PropTypes.func.isRequired,
};

const defaultProps = {
rowHeight: null,
};

class Row extends React.PureComponent {
constructor(props) {
super(props);
Expand Down Expand Up @@ -192,6 +188,5 @@ class Row extends React.PureComponent {
}

Row.propTypes = propTypes;
Row.defaultProps = defaultProps;

export default Row;
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ const propTypes = {
};

const defaultProps = {
children: null,
renderTabContent: true,
renderHoverMenu: true,
availableColumnCount: 0,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ const defaultProps = {
children: null,
disableClick: false,
onChangeFocus: null,
onPressDelete() {},
menuItems: [],
isFocused: false,
shouldFocus: (event, container) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import { connect } from 'react-redux';
import DashboardBuilder from '../components/DashboardBuilder';

import {
setColorSchemeAndUnsavedChanges,
showBuilderPane,
setDirectPathToChild,
setMountedTab,
Expand All @@ -35,9 +34,7 @@ function mapStateToProps({ dashboardLayout: undoableLayout, dashboardState }) {
return {
dashboardLayout: undoableLayout.present,
editMode: dashboardState.editMode,
showBuilderPane: dashboardState.showBuilderPane,
directPathToChild: dashboardState.directPathToChild,
colorScheme: dashboardState.colorScheme,
};
}

Expand All @@ -47,7 +44,6 @@ function mapDispatchToProps(dispatch) {
deleteTopLevelTabs,
handleComponentDrop,
showBuilderPane,
setColorSchemeAndUnsavedChanges,
setDirectPathToChild,
setMountedTab,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ const propTypes = {
const defaultProps = {
directPathToChild: [],
directPathLastUpdated: 0,
isComponentVisible: true,
};

function mapStateToProps(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ const propTypes = {
const defaultProps = {
onStop: () => {},
onSave: () => {},
disabled: false,
};

// Prolly need to move this to a global context
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ const propTypes = {
choices: PropTypes.oneOfType([
PropTypes.arrayOf(PropTypes.array),
PropTypes.func,
]).isRequired,
schemes: PropTypes.oneOfType([PropTypes.object, PropTypes.func]).isRequired,
]),
schemes: PropTypes.oneOfType([PropTypes.object, PropTypes.func]),
isLinear: PropTypes.bool,
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export const DEFAULT_VIEWPORT = {
const PARAMS = ['longitude', 'latitude', 'zoom', 'bearing', 'pitch'];

const propTypes = {
onChange: PropTypes.func.isRequired,
onChange: PropTypes.func,
value: PropTypes.shape({
longitude: PropTypes.number,
latitude: PropTypes.number,
Expand Down