Skip to content

Commit cc51b20

Browse files
authored
refactor(QueryEditor): get rid of some misused useEffects (#1107)
1 parent 70b6dc3 commit cc51b20

File tree

4 files changed

+116
-146
lines changed

4 files changed

+116
-146
lines changed

src/containers/Tenant/Query/QueryEditor/QueryEditor.tsx

+96-144
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ import {
3131
LAST_USED_QUERY_ACTION_KEY,
3232
QUERY_USE_MULTI_SCHEMA_KEY,
3333
} from '../../../../utils/constants';
34-
import {useQueryExecutionSettings, useSetting} from '../../../../utils/hooks';
34+
import {useEventHandler, useQueryExecutionSettings, useSetting} from '../../../../utils/hooks';
3535
import {useChangedQuerySettings} from '../../../../utils/hooks/useChangedQuerySettings';
3636
import {useLastQueryExecutionSettings} from '../../../../utils/hooks/useLastQueryExecutionSettings';
3737
import {YQL_LANGUAGE_ID} from '../../../../utils/monaco/constats';
@@ -59,10 +59,6 @@ const RESULT_TYPES = {
5959
EXECUTE: 'execute',
6060
EXPLAIN: 'explain',
6161
};
62-
const MONACO_HOT_KEY_ACTIONS = {
63-
sendQuery: 'sendQuery',
64-
sendSelectedQuery: 'sendSelectedQuery',
65-
};
6662

6763
const b = cn('query-editor');
6864

@@ -94,9 +90,6 @@ function QueryEditor(props: QueryEditorProps) {
9490
tenantName,
9591
path,
9692
setTenantPath: setPath,
97-
setQueryAction,
98-
saveQueryToHistory,
99-
setShowPreview,
10093
executeQuery,
10194
type,
10295
theme,
@@ -117,9 +110,6 @@ function QueryEditor(props: QueryEditorProps) {
117110
const [lastUsedQueryAction, setLastUsedQueryAction] = useSetting<QueryAction>(
118111
LAST_USED_QUERY_ACTION_KEY,
119112
);
120-
const [monacoHotKey, setMonacoHotKey] = React.useState<ValueOf<
121-
typeof MONACO_HOT_KEY_ACTIONS
122-
> | null>(null);
123113

124114
const [sendExecuteQuery, executeQueryResult] = executeQueryApi.useExecuteQueryMutation();
125115
const [sendExplainQuery, explainQueryResult] = explainQueryApi.useExplainQueryMutation();
@@ -138,27 +128,6 @@ function QueryEditor(props: QueryEditorProps) {
138128
initialTenantCommonInfoState,
139129
);
140130

141-
const editorRef = React.useRef<Monaco.editor.IStandaloneCodeEditor>();
142-
143-
React.useEffect(() => {
144-
const updateEditor = () => {
145-
if (editorRef.current) {
146-
editorRef.current.layout();
147-
}
148-
};
149-
150-
const onChangeWindow = throttle(() => {
151-
updateEditor();
152-
}, 100);
153-
154-
updateEditor();
155-
156-
window.addEventListener('resize', onChangeWindow);
157-
return () => {
158-
window.removeEventListener('resize', onChangeWindow);
159-
};
160-
}, []);
161-
162131
React.useEffect(() => {
163132
dispatchResultVisibilityState(PaneVisibilityActionTypes.triggerCollapse);
164133
}, []);
@@ -171,83 +140,51 @@ function QueryEditor(props: QueryEditorProps) {
171140
}
172141
}, [props.showPreview, isResultLoaded]);
173142

174-
React.useEffect(() => {
175-
const {input, history} = executeQuery;
143+
const getLastQueryText = useEventHandler(() => {
144+
const {history} = executeQuery;
145+
return history.queries[history.queries.length - 1]?.queryText || '';
146+
});
176147

177-
const hasUnsavedInput = input
178-
? input !== history.queries[history.queries.length - 1]?.queryText
179-
: false;
180-
181-
if (hasUnsavedInput) {
182-
window.onbeforeunload = (e) => {
183-
e.preventDefault();
184-
// Chrome requires returnValue to be set
185-
e.returnValue = '';
186-
};
187-
} else {
188-
window.onbeforeunload = null;
189-
}
190-
return () => {
191-
window.onbeforeunload = null;
192-
};
193-
}, [executeQuery]);
148+
const handleSendExecuteClick = useEventHandler((text?: string) => {
149+
const {input, history} = executeQuery;
194150

195-
const handleSendExecuteClick = React.useCallback(
196-
(text?: string) => {
197-
const {input, history} = executeQuery;
151+
const schema = useMultiSchema ? 'multi' : 'modern';
198152

199-
const schema = useMultiSchema ? 'multi' : 'modern';
153+
const query = text && typeof text === 'string' ? text : input;
200154

201-
const query = text && typeof text === 'string' ? text : input;
155+
setLastUsedQueryAction(QUERY_ACTIONS.execute);
156+
if (!isEqual(lastQueryExecutionSettings, querySettings)) {
157+
resetBanner();
158+
setLastQueryExecutionSettings(querySettings);
159+
}
202160

203-
setLastUsedQueryAction(QUERY_ACTIONS.execute);
204-
if (!isEqual(lastQueryExecutionSettings, querySettings)) {
205-
resetBanner();
206-
setLastQueryExecutionSettings(querySettings);
207-
}
161+
setResultType(RESULT_TYPES.EXECUTE);
162+
sendExecuteQuery({
163+
query,
164+
database: tenantName,
165+
querySettings,
166+
schema,
167+
enableTracingLevel,
168+
});
169+
setIsResultLoaded(true);
170+
props.setShowPreview(false);
208171

209-
setResultType(RESULT_TYPES.EXECUTE);
210-
sendExecuteQuery({
211-
query,
212-
database: tenantName,
213-
querySettings,
214-
schema,
215-
enableTracingLevel,
216-
});
217-
setIsResultLoaded(true);
218-
setShowPreview(false);
219-
220-
// Don't save partial queries in history
221-
if (!text) {
222-
const {queries, currentIndex} = history;
223-
if (query !== queries[currentIndex]?.queryText) {
224-
saveQueryToHistory(input);
225-
}
172+
// Don't save partial queries in history
173+
if (!text) {
174+
const {queries, currentIndex} = history;
175+
if (query !== queries[currentIndex]?.queryText) {
176+
props.saveQueryToHistory(input);
226177
}
227-
dispatchResultVisibilityState(PaneVisibilityActionTypes.triggerExpand);
228-
},
229-
[
230-
executeQuery,
231-
enableTracingLevel,
232-
useMultiSchema,
233-
setLastUsedQueryAction,
234-
lastQueryExecutionSettings,
235-
querySettings,
236-
sendExecuteQuery,
237-
saveQueryToHistory,
238-
setShowPreview,
239-
tenantName,
240-
resetBanner,
241-
setLastQueryExecutionSettings,
242-
],
243-
);
178+
}
179+
dispatchResultVisibilityState(PaneVisibilityActionTypes.triggerExpand);
180+
});
244181

245182
const handleSettingsClick = () => {
246-
setQueryAction('settings');
183+
props.setQueryAction('settings');
247184
props.setShowPreview(false);
248185
};
249186

250-
const handleGetExplainQueryClick = React.useCallback(() => {
187+
const handleGetExplainQueryClick = useEventHandler(() => {
251188
const {input} = executeQuery;
252189

253190
setLastUsedQueryAction(QUERY_ACTIONS.explain);
@@ -265,56 +202,22 @@ function QueryEditor(props: QueryEditorProps) {
265202
enableTracingLevel,
266203
});
267204
setIsResultLoaded(true);
268-
setShowPreview(false);
205+
props.setShowPreview(false);
269206
dispatchResultVisibilityState(PaneVisibilityActionTypes.triggerExpand);
270-
}, [
271-
executeQuery,
272-
lastQueryExecutionSettings,
273-
querySettings,
274-
resetBanner,
275-
enableTracingLevel,
276-
sendExplainQuery,
277-
setLastQueryExecutionSettings,
278-
setLastUsedQueryAction,
279-
setShowPreview,
280-
tenantName,
281-
]);
207+
});
282208

283-
React.useEffect(() => {
284-
if (monacoHotKey === null) {
285-
return;
286-
}
287-
setMonacoHotKey(null);
288-
switch (monacoHotKey) {
289-
case MONACO_HOT_KEY_ACTIONS.sendQuery: {
290-
if (lastUsedQueryAction === QUERY_ACTIONS.explain) {
291-
handleGetExplainQueryClick();
292-
} else {
293-
handleSendExecuteClick();
294-
}
295-
break;
296-
}
297-
case MONACO_HOT_KEY_ACTIONS.sendSelectedQuery: {
298-
const selection = editorRef.current?.getSelection();
299-
const model = editorRef.current?.getModel();
300-
if (selection && model) {
301-
const text = model.getValueInRange({
302-
startLineNumber: selection.getSelectionStart().lineNumber,
303-
startColumn: selection.getSelectionStart().column,
304-
endLineNumber: selection.getPosition().lineNumber,
305-
endColumn: selection.getPosition().column,
306-
});
307-
handleSendExecuteClick(text);
308-
}
309-
break;
310-
}
209+
const handleSendQuery = useEventHandler(() => {
210+
if (lastUsedQueryAction === QUERY_ACTIONS.explain) {
211+
handleGetExplainQueryClick();
212+
} else {
213+
handleSendExecuteClick();
311214
}
312-
// eslint-disable-next-line react-hooks/exhaustive-deps
313-
}, [monacoHotKey, handleSendExecuteClick, handleGetExplainQueryClick]);
215+
});
314216

315217
const editorDidMount = (editor: Monaco.editor.IStandaloneCodeEditor, monaco: typeof Monaco) => {
316218
const keybindings = getKeyBindings(monaco);
317-
editorRef.current = editor;
219+
initResizeHandler(editor);
220+
initUserPrompt(editor, getLastQueryText);
318221
editor.focus();
319222
editor.addAction({
320223
id: 'sendQuery',
@@ -328,7 +231,7 @@ function QueryEditor(props: QueryEditorProps) {
328231
contextMenuOrder: 1,
329232
// Method that will be executed when the action is triggered.
330233
// @param editor The editor instance is passed in as a convenience
331-
run: () => setMonacoHotKey(MONACO_HOT_KEY_ACTIONS.sendQuery),
234+
run: () => handleSendQuery(),
332235
});
333236

334237
const canSendSelectedText = editor.createContextKey<boolean>('canSendSelectedText', false);
@@ -346,7 +249,19 @@ function QueryEditor(props: QueryEditorProps) {
346249
precondition: 'canSendSelectedText',
347250
contextMenuGroupId: CONTEXT_MENU_GROUP_ID,
348251
contextMenuOrder: 1,
349-
run: () => setMonacoHotKey(MONACO_HOT_KEY_ACTIONS.sendSelectedQuery),
252+
run: (e) => {
253+
const selection = e.getSelection();
254+
const model = e.getModel();
255+
if (selection && model) {
256+
const text = model.getValueInRange({
257+
startLineNumber: selection.getSelectionStart().lineNumber,
258+
startColumn: selection.getSelectionStart().column,
259+
endLineNumber: selection.getPosition().lineNumber,
260+
endColumn: selection.getPosition().column,
261+
});
262+
handleSendExecuteClick(text);
263+
}
264+
},
350265
});
351266

352267
editor.addAction({
@@ -374,7 +289,7 @@ function QueryEditor(props: QueryEditorProps) {
374289
label: i18n('action.save-query'),
375290
keybindings: [keybindings.saveQuery],
376291
run: () => {
377-
setQueryAction('save');
292+
props.setQueryAction('save');
378293
},
379294
});
380295
};
@@ -554,3 +469,40 @@ function Result({
554469

555470
return null;
556471
}
472+
473+
function initResizeHandler(editor: Monaco.editor.IStandaloneCodeEditor) {
474+
const layoutEditor = throttle(() => {
475+
editor.layout();
476+
}, 100);
477+
478+
editor.layout();
479+
480+
window.addEventListener('resize', layoutEditor);
481+
editor.onDidDispose(() => {
482+
window.removeEventListener('resize', layoutEditor);
483+
});
484+
}
485+
486+
function initUserPrompt(editor: Monaco.editor.IStandaloneCodeEditor, getInitialText: () => string) {
487+
setUserPrompt(editor.getValue(), getInitialText());
488+
editor.onDidChangeModelContent(() => {
489+
setUserPrompt(editor.getValue(), getInitialText());
490+
});
491+
editor.onDidDispose(() => {
492+
window.onbeforeunload = null;
493+
});
494+
}
495+
496+
function setUserPrompt(text: string, initialText: string) {
497+
const hasUnsavedInput = text ? text !== initialText : false;
498+
499+
if (hasUnsavedInput) {
500+
window.onbeforeunload = (e) => {
501+
e.preventDefault();
502+
// Chrome requires returnValue to be set
503+
e.returnValue = '';
504+
};
505+
} else {
506+
window.onbeforeunload = null;
507+
}
508+
}

src/utils/constants.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -122,13 +122,13 @@ export const TENANT_OVERVIEW_TABLES_SETTINGS = {
122122
dynamicRender: false,
123123
} as const;
124124

125-
export const DEFAULT_QUERY_SETTINGS: QuerySettings = {
125+
export const DEFAULT_QUERY_SETTINGS = {
126126
queryMode: QUERY_MODES.script,
127127
isolationLevel: ISOLATION_LEVELS.serializable,
128128
timeout: '60',
129129
statisticsMode: STATISTICS_MODES.none,
130130
tracingLevel: TRACING_LEVELS.detailed,
131-
};
131+
} satisfies QuerySettings;
132132

133133
export const QUERY_EXECUTION_SETTINGS_KEY = 'queryExecutionSettings';
134134
export const LAST_QUERY_EXECUTION_SETTINGS_KEY = 'last_query_execution_settings';

src/utils/hooks/index.ts

+1
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,4 @@ export * from './useQueryExecutionSettings';
55
export * from './useTableSort';
66
export * from './useSearchQuery';
77
export * from './useAutoRefreshInterval';
8+
export * from './useEventHandler';

src/utils/hooks/useEventHandler.ts

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import React from 'react';
2+
3+
/**
4+
* The hook returns a stable function (an empty list of dependencies),
5+
* but this function always calls the actual function associated with the last render.
6+
* The returned function should be used as an event handler or inside a useEffect.
7+
*/
8+
export function useEventHandler<T extends Function>(handler?: T) {
9+
const ref = React.useRef<T | undefined>(handler);
10+
React.useLayoutEffect(() => {
11+
ref.current = handler;
12+
}, [handler]);
13+
// @ts-expect-error
14+
return React.useCallback<T>((...args) => {
15+
return ref.current?.(...args);
16+
}, []);
17+
}

0 commit comments

Comments
 (0)