Skip to content

Commit

Permalink
inline eslint rule for difficult cases
Browse files Browse the repository at this point in the history
  • Loading branch information
mbondyra committed Jul 30, 2020
1 parent 3d974dd commit efba090
Show file tree
Hide file tree
Showing 6 changed files with 243 additions and 216 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
106 changes: 55 additions & 51 deletions x-pack/plugins/lens/public/app_plugin/app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -216,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 @@ -62,34 +62,38 @@ export function EditorFrame(props: EditorFrameProps) {
);

// Initialize current datasource and all active datasources
useEffect(() => {
// prevents executing dispatch on unmounted component
let isUnmounted = false;
if (!allLoaded) {
Object.entries(props.datasourceMap).forEach(([datasourceId, datasource]) => {
if (
state.datasourceStates[datasourceId] &&
state.datasourceStates[datasourceId].isLoading
) {
datasource
.initialize(state.datasourceStates[datasourceId].state || undefined)
.then((datasourceState) => {
if (!isUnmounted) {
dispatch({
type: 'UPDATE_DATASOURCE_STATE',
updater: datasourceState,
datasourceId,
});
}
})
.catch(onError);
}
});
}
return () => {
isUnmounted = true;
};
}, [allLoaded]);
useEffect(
() => {
// prevents executing dispatch on unmounted component
let isUnmounted = false;
if (!allLoaded) {
Object.entries(props.datasourceMap).forEach(([datasourceId, datasource]) => {
if (
state.datasourceStates[datasourceId] &&
state.datasourceStates[datasourceId].isLoading
) {
datasource
.initialize(state.datasourceStates[datasourceId].state || undefined)
.then((datasourceState) => {
if (!isUnmounted) {
dispatch({
type: 'UPDATE_DATASOURCE_STATE',
updater: datasourceState,
datasourceId,
});
}
})
.catch(onError);
}
});
}
return () => {
isUnmounted = true;
};
},
// eslint-disable-next-line react-hooks/exhaustive-deps
[allLoaded, onError]
);

const datasourceLayers: Record<string, DatasourcePublicAPI> = {};
Object.keys(props.datasourceMap)
Expand Down Expand Up @@ -156,83 +160,95 @@ export function EditorFrame(props: EditorFrameProps) {
},
};

useEffect(() => {
if (props.doc) {
dispatch({
type: 'VISUALIZATION_LOADED',
doc: props.doc,
});
} else {
dispatch({
type: 'RESET',
state: getInitialState(props),
});
}
}, [props.doc]);
useEffect(
() => {
if (props.doc) {
dispatch({
type: 'VISUALIZATION_LOADED',
doc: props.doc,
});
} else {
dispatch({
type: 'RESET',
state: getInitialState(props),
});
}
},
// eslint-disable-next-line react-hooks/exhaustive-deps
[props.doc]
);

// Initialize visualization as soon as all datasources are ready
useEffect(() => {
if (allLoaded && state.visualization.state === null && activeVisualization) {
const initialVisualizationState = activeVisualization.initialize(framePublicAPI);
dispatch({
type: 'UPDATE_VISUALIZATION_STATE',
visualizationId: activeVisualization.id,
newState: initialVisualizationState,
});
}
}, [allLoaded, activeVisualization, state.visualization.state]);
useEffect(
() => {
if (allLoaded && state.visualization.state === null && activeVisualization) {
const initialVisualizationState = activeVisualization.initialize(framePublicAPI);
dispatch({
type: 'UPDATE_VISUALIZATION_STATE',
visualizationId: activeVisualization.id,
newState: initialVisualizationState,
});
}
},
// eslint-disable-next-line react-hooks/exhaustive-deps
[allLoaded, activeVisualization, state.visualization.state]
);

// The frame needs to call onChange every time its internal state changes
useEffect(() => {
const activeDatasource =
state.activeDatasourceId && !state.datasourceStates[state.activeDatasourceId].isLoading
? props.datasourceMap[state.activeDatasourceId]
: undefined;
useEffect(
() => {
const activeDatasource =
state.activeDatasourceId && !state.datasourceStates[state.activeDatasourceId].isLoading
? props.datasourceMap[state.activeDatasourceId]
: undefined;

if (!activeDatasource || !activeVisualization) {
return;
}
if (!activeDatasource || !activeVisualization) {
return;
}

const indexPatterns: DatasourceMetaData['filterableIndexPatterns'] = [];
Object.entries(props.datasourceMap)
.filter(([id, datasource]) => {
const stateWrapper = state.datasourceStates[id];
return (
stateWrapper &&
!stateWrapper.isLoading &&
datasource.getLayers(stateWrapper.state).length > 0
);
})
.forEach(([id, datasource]) => {
indexPatterns.push(
...datasource.getMetaData(state.datasourceStates[id].state).filterableIndexPatterns
);
});
const indexPatterns: DatasourceMetaData['filterableIndexPatterns'] = [];
Object.entries(props.datasourceMap)
.filter(([id, datasource]) => {
const stateWrapper = state.datasourceStates[id];
return (
stateWrapper &&
!stateWrapper.isLoading &&
datasource.getLayers(stateWrapper.state).length > 0
);
})
.forEach(([id, datasource]) => {
indexPatterns.push(
...datasource.getMetaData(state.datasourceStates[id].state).filterableIndexPatterns
);
});

const doc = getSavedObjectFormat({
activeDatasources: Object.keys(state.datasourceStates).reduce(
(datasourceMap, datasourceId) => ({
...datasourceMap,
[datasourceId]: props.datasourceMap[datasourceId],
}),
{}
),
visualization: activeVisualization,
state,
framePublicAPI,
});
const doc = getSavedObjectFormat({
activeDatasources: Object.keys(state.datasourceStates).reduce(
(datasourceMap, datasourceId) => ({
...datasourceMap,
[datasourceId]: props.datasourceMap[datasourceId],
}),
{}
),
visualization: activeVisualization,
state,
framePublicAPI,
});

props.onChange({ filterableIndexPatterns: indexPatterns, doc });
}, [
activeVisualization,
state.datasourceStates,
state.visualization,
props.query,
props.dateRange,
props.filters,
props.savedQuery,
state.title,
]);
props.onChange({ filterableIndexPatterns: indexPatterns, doc });
},
// eslint-disable-next-line react-hooks/exhaustive-deps
[
activeVisualization,
state.datasourceStates,
state.visualization,
props.query,
props.dateRange,
props.filters,
props.savedQuery,
state.title,
]
);

return (
<RootDragDropProvider>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ export function ChartSwitch(props: Props) {
...visualizationType,
selection: getSelection(visualizationType.visualizationId, visualizationType.id),
})),
// eslint-disable-next-line react-hooks/exhaustive-deps
[
flyoutOpen,
props.visualizationMap,
Expand Down
Loading

0 comments on commit efba090

Please sign in to comment.