CB-4468 iterate over copy of state array#2278
Conversation
| for (const tab of state.tabs) { | ||
| const tabs = state.tabs.slice(); | ||
|
|
||
| for (const tab of tabs) { |
There was a problem hiding this comment.
I suggest to do the same thing for this function below. Just to avoid similar bug in future when the logic gets a bit more complex:
async canCloseResultTabs(state: ISqlEditorTabState): Promise<boolean> {
const tabs = state.tabs.slice();
for (const tab of tabs) {
const canClose = await this.sqlQueryResultService.canCloseResultTab(state, tab.id);
if (!canClose) {
return false;
}
}
return true;
}
There was a problem hiding this comment.
This one is a bit tricky because of asynchronous.
Imagine a case when we clicked to close all tabs
- First, we will check that every tab can be closed
- for example, let's assume that one tab can close fn took more than a 1s
- we can try to close any other tab at this moment
This will create a case when a tab in the tabs array is closed.
So, in general, canCloseResultTabs is not affected by this bug, but I described another case before. So, we probably need to track such operations and disable related actions during transactions.
| for (const tab of state.tabs) { | ||
| const tabs = state.tabs.slice(); | ||
|
|
||
| for (const tab of tabs) { |
There was a problem hiding this comment.
This one is a bit tricky because of asynchronous.
Imagine a case when we clicked to close all tabs
- First, we will check that every tab can be closed
- for example, let's assume that one tab can close fn took more than a 1s
- we can try to close any other tab at this moment
This will create a case when a tab in the tabs array is closed.
So, in general, canCloseResultTabs is not affected by this bug, but I described another case before. So, we probably need to track such operations and disable related actions during transactions.
No description provided.