Skip to content

Commit

Permalink
fix: do not clear dialog state immediately on useDialog unmount
Browse files Browse the repository at this point in the history
  • Loading branch information
myandrienko committed Dec 18, 2024
1 parent 56def19 commit 13c9c18
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 6 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@
"react-dom": "^18.1.0",
"react-test-renderer": "^18.1.0",
"semantic-release": "^19.0.5",
"stream-chat": "^8.46.1",
"stream-chat": "^8.47.1",
"ts-jest": "^29.1.4",
"typescript": "^5.4.5"
},
Expand Down
49 changes: 49 additions & 0 deletions src/components/Dialog/DialogManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export type Dialog = {
id: DialogId;
isOpen: boolean | undefined;
open: (zIndex?: number) => void;
removalTimeout: NodeJS.Timeout | undefined;
remove: () => void;
toggle: (closeAll?: boolean) => void;
};
Expand Down Expand Up @@ -64,6 +65,7 @@ export class DialogManager {
open: () => {
this.open({ id });
},
removalTimeout: undefined,
remove: () => {
this.remove(id);
},
Expand All @@ -76,6 +78,23 @@ export class DialogManager {
...{ dialogsById: { ...current.dialogsById, [id]: dialog } },
}));
}

if (dialog.removalTimeout) {
clearTimeout(dialog.removalTimeout);
this.state.next((current) => ({
...current,
...{
dialogsById: {
...current.dialogsById,
[id]: {
...dialog,
removalTimeout: undefined,
},
},
},
}));
}

return dialog;
}

Expand Down Expand Up @@ -117,6 +136,10 @@ export class DialogManager {
const dialog = state.dialogsById[id];
if (!dialog) return;

if (dialog.removalTimeout) {
clearTimeout(dialog.removalTimeout);
}

this.state.next((current) => {
const newDialogs = { ...current.dialogsById };
delete newDialogs[id];
Expand All @@ -126,4 +149,30 @@ export class DialogManager {
};
});
}

/**
* Marks the dialog state as unused. If the dialog id is referenced again quickly,
* the state will not be removed. Otherwise, the state will be removed after
* a short timeout.
*/
markForRemoval(id: DialogId) {
const dialog = this.state.getLatestValue().dialogsById[id];

if (!dialog) {
return;
}

this.state.next((current) => ({
...current,
dialogsById: {
...current.dialogsById,
[id]: {
...dialog,
removalTimeout: setTimeout(() => {
this.remove(id);
}, 16),
},
},
}));
}
}
6 changes: 5 additions & 1 deletion src/components/Dialog/hooks/useDialog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@ export const useDialog = ({ id }: GetOrCreateDialogParams) => {

useEffect(
() => () => {
dialogManager.remove(id);
// Since this cleanup can run even if the component is still mounted
// and dialog id is unchanged (e.g. in <StrictMode />), it's safer to
// mark state as unused and only remove it after a timeout, rather than
// to remove it immediately.
dialogManager.markForRemoval(id);
},
[dialogManager, id],
);
Expand Down
8 changes: 4 additions & 4 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -12231,10 +12231,10 @@ [email protected]:
resolved "https://registry.yarnpkg.com/statuses/-/statuses-2.0.1.tgz#55cb000ccf1d48728bd23c685a063998cf1a1b63"
integrity sha512-RwNA9Z/7PrK06rYLIzFMlaF+l73iwpzsqRIFgbMLbTcLD6cOao82TaWefPXQvB2fOC4AjuYSEndS7N/mTCbkdQ==

stream-chat@^8.46.1:
version "8.46.1"
resolved "https://registry.yarnpkg.com/stream-chat/-/stream-chat-8.46.1.tgz#9624bbd4e8e357414389e4b0dcb5f0134487f679"
integrity sha512-jVg148tZDCAmZa6b31cnamiUvt58IQB4YTRIGFt3+4zp6jI2tKeHvEj24uEcVF2d7fhT1IXSuGhNfcrStqSZHQ==
stream-chat@^8.47.1:
version "8.47.1"
resolved "https://registry.yarnpkg.com/stream-chat/-/stream-chat-8.47.1.tgz#5390c87cbb1929e7ca183aa1204dae3ab38469a2"
integrity sha512-raMAGYLT4UCVluMF0TMfdPKH9OUhDjH6e1HQdJIlllAFLaA8oxtG+e/7jyuPmVodLPzYCPqOt2eBH7soAkhV/A==
dependencies:
"@babel/runtime" "^7.16.3"
"@types/jsonwebtoken" "~9.0.0"
Expand Down

0 comments on commit 13c9c18

Please sign in to comment.