Skip to content

Commit

Permalink
[Lens] Clean and inline disabling of react-hooks/exhaustive-deps esli…
Browse files Browse the repository at this point in the history
…nt rule (#70010)
  • Loading branch information
mbondyra authored Aug 6, 2020
1 parent aa75f80 commit 626fbc2
Show file tree
Hide file tree
Showing 15 changed files with 285 additions and 241 deletions.
6 changes: 0 additions & 6 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,12 +132,6 @@ module.exports = {
'react-hooks/rules-of-hooks': 'off',
},
},
{
files: ['x-pack/plugins/lens/**/*.{js,mjs,ts,tsx}'],
rules: {
'react-hooks/exhaustive-deps': 'off',
},
},
{
files: ['x-pack/plugins/ml/**/*.{js,mjs,ts,tsx}'],
rules: {
Expand Down
114 changes: 62 additions & 52 deletions x-pack/plugins/lens/public/app_plugin/app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,13 @@ export function App({
filterSubscription.unsubscribe();
timeSubscription.unsubscribe();
};
}, [data.query.filterManager, data.query.timefilter.timefilter]);
}, [
data.query.filterManager,
data.query.timefilter.timefilter,
core.uiSettings,
data.query,
history,
]);

useEffect(() => {
onAppLeave((actions) => {
Expand Down Expand Up @@ -210,57 +216,61 @@ export function App({
]);
}, [core.application, core.chrome, core.http.basePath, state.persistedDoc]);

useEffect(() => {
if (docId && (!state.persistedDoc || state.persistedDoc.id !== docId)) {
setState((s) => ({ ...s, isLoading: true }));
docStorage
.load(docId)
.then((doc) => {
getAllIndexPatterns(
doc.state.datasourceMetaData.filterableIndexPatterns,
data.indexPatterns,
core.notifications
)
.then((indexPatterns) => {
// Don't overwrite any pinned filters
data.query.filterManager.setAppFilters(doc.state.filters);
setState((s) => ({
...s,
isLoading: false,
persistedDoc: doc,
lastKnownDoc: doc,
query: doc.state.query,
indexPatternsForTopNav: indexPatterns,
}));
})
.catch(() => {
setState((s) => ({ ...s, isLoading: false }));

redirectTo();
});
})
.catch(() => {
setState((s) => ({ ...s, isLoading: false }));

core.notifications.toasts.addDanger(
i18n.translate('xpack.lens.app.docLoadingError', {
defaultMessage: 'Error loading saved document',
})
);

redirectTo();
});
}
}, [
core.notifications,
data.indexPatterns,
data.query.filterManager,
docId,
// TODO: These dependencies are changing too often
// docStorage,
// redirectTo,
// state.persistedDoc,
]);
useEffect(
() => {
if (docId && (!state.persistedDoc || state.persistedDoc.id !== docId)) {
setState((s) => ({ ...s, isLoading: true }));
docStorage
.load(docId)
.then((doc) => {
getAllIndexPatterns(
doc.state.datasourceMetaData.filterableIndexPatterns,
data.indexPatterns,
core.notifications
)
.then((indexPatterns) => {
// Don't overwrite any pinned filters
data.query.filterManager.setAppFilters(doc.state.filters);
setState((s) => ({
...s,
isLoading: false,
persistedDoc: doc,
lastKnownDoc: doc,
query: doc.state.query,
indexPatternsForTopNav: indexPatterns,
}));
})
.catch(() => {
setState((s) => ({ ...s, isLoading: false }));

redirectTo();
});
})
.catch(() => {
setState((s) => ({ ...s, isLoading: false }));

core.notifications.toasts.addDanger(
i18n.translate('xpack.lens.app.docLoadingError', {
defaultMessage: 'Error loading saved document',
})
);

redirectTo();
});
}
},
// eslint-disable-next-line react-hooks/exhaustive-deps
[
core.notifications,
data.indexPatterns,
data.query.filterManager,
docId,
// TODO: These dependencies are changing too often
// docStorage,
// redirectTo,
// state.persistedDoc,
]
);

const runSave = async (
saveProps: Omit<OnSaveProps, 'onTitleDuplicate' | 'newDescription'> & {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ export function DatatableComponent(props: DatatableRenderProps) {
formatters[column.id] = props.formatFactory(column.formatHint);
});

const { onClickValue } = props;
const handleFilterClick = useMemo(
() => (field: string, value: unknown, colIndex: number, negate: boolean = false) => {
const col = firstTable.columns[colIndex];
Expand All @@ -180,9 +181,9 @@ export function DatatableComponent(props: DatatableRenderProps) {
],
timeFieldName,
};
props.onClickValue(desanitizeFilterContext(data));
onClickValue(desanitizeFilterContext(data));
},
[firstTable]
[firstTable, onClickValue]
);

const bucketColumns = firstTable.columns
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,11 @@ export function debouncedComponent<TProps>(component: FunctionComponent<TProps>,

return (props: TProps) => {
const [cachedProps, setCachedProps] = useState(props);
const debouncePropsChange = debounce(setCachedProps, delay);
const delayRender = useMemo(() => debouncePropsChange, []);
const debouncePropsChange = useMemo(() => debounce(setCachedProps, delay), [setCachedProps]);

// cancel debounced prop change if component has been unmounted in the meantime
useEffect(() => () => debouncePropsChange.cancel(), []);

delayRender(props);
useEffect(() => () => debouncePropsChange.cancel(), [debouncePropsChange]);
debouncePropsChange(props);

return React.createElement(MemoizedComponent, cachedProps);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,29 +39,29 @@ function LayerPanels(
} = props;
const setVisualizationState = useMemo(
() => (newState: unknown) => {
props.dispatch({
dispatch({
type: 'UPDATE_VISUALIZATION_STATE',
visualizationId: activeVisualization.id,
newState,
clearStagedPreview: false,
});
},
[props.dispatch, activeVisualization]
[dispatch, activeVisualization]
);
const updateDatasource = useMemo(
() => (datasourceId: string, newState: unknown) => {
props.dispatch({
dispatch({
type: 'UPDATE_DATASOURCE_STATE',
updater: () => newState,
datasourceId,
clearStagedPreview: false,
});
},
[props.dispatch]
[dispatch]
);
const updateAll = useMemo(
() => (datasourceId: string, newDatasourceState: unknown, newVisualizationState: unknown) => {
props.dispatch({
dispatch({
type: 'UPDATE_STATE',
subType: 'UPDATE_ALL_STATES',
updater: (prevState) => {
Expand All @@ -83,7 +83,7 @@ function LayerPanels(
},
});
},
[props.dispatch]
[dispatch]
);
const layerIds = activeVisualization.getLayerIds(visualizationState);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,17 @@ interface DataPanelWrapperProps {
}

export const DataPanelWrapper = memo((props: DataPanelWrapperProps) => {
const { dispatch, activeDatasource } = props;
const setDatasourceState: StateSetter<unknown> = useMemo(
() => (updater) => {
props.dispatch({
dispatch({
type: 'UPDATE_DATASOURCE_STATE',
updater,
datasourceId: props.activeDatasource!,
datasourceId: activeDatasource!,
clearStagedPreview: true,
});
},
[props.dispatch, props.activeDatasource]
[dispatch, activeDatasource]
);

const datasourceProps: DatasourceDataPanelProps = {
Expand Down
Loading

0 comments on commit 626fbc2

Please sign in to comment.