Skip to content

Commit 9fd5ce0

Browse files
authored
experimental: Abort controller (#4195)
## Description I tried how the code will look like with abort controller, if you don't like it - we can close this without merging ## Steps for reproduction 1. click button 2. expect xyz ## Code Review - [ ] hi @kof, I need you to do - conceptual review (architecture, feature-correctness) - detailed review (read every line) - test it on preview ## Before requesting a review - [ ] made a self-review - [ ] added inline comments where things may be not obvious (the "why", not "what") ## Before merging - [ ] tested locally and on preview environment (preview dev login: 5de6) - [ ] updated [test cases](https://github.com/webstudio-is/webstudio/blob/main/apps/builder/docs/test-cases.md) document - [ ] added tests - [ ] if any new env variables are added, added them to `.env` file
1 parent 22e8f2b commit 9fd5ce0

14 files changed

+156
-134
lines changed

apps/builder/app/builder/builder.tsx

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ import {
3232
} from "~/shared/nano-states";
3333
import { $settings, type Settings } from "./shared/client-settings";
3434
import { builderUrl, getCanvasUrl } from "~/shared/router-utils";
35-
import { useCopyPaste } from "~/shared/copy-paste";
3635
import { BlockingAlerts } from "./features/blocking-alerts";
3736
import { useSyncPageUrl } from "~/shared/pages";
3837
import { useMount, useUnmount } from "~/shared/hook-utils/use-mount";
@@ -58,6 +57,7 @@ import { updateWebstudioData } from "~/shared/instance-utils";
5857
import { migrateWebstudioDataMutable } from "~/shared/webstudio-data-migrator";
5958
import { Loading, LoadingBackground } from "./shared/loading";
6059
import { mergeRefs } from "@react-aria/utils";
60+
import { initCopyPaste } from "~/shared/copy-paste";
6161

6262
registerContainers();
6363

@@ -285,9 +285,7 @@ export const Builder = ({
285285
const isCloneDialogOpen = useStore($isCloneDialogOpen);
286286
const isPreviewMode = useStore($isPreviewMode);
287287
const { onRef: onRefReadCanvas, onTransitionEnd } = useReadCanvasRect();
288-
// We need to initialize this in both canvas and builder,
289-
// because the events will fire in either one, depending on where the focus is
290-
useCopyPaste();
288+
291289
useSetWindowTitle();
292290

293291
const iframeRefCallback = mergeRefs((element: HTMLIFrameElement | null) => {
@@ -299,14 +297,24 @@ export const Builder = ({
299297
const [loadingState, setLoadingState] = useState(() => $loadingState.get());
300298

301299
useEffect(() => {
300+
const abortController = new AbortController();
301+
// We need to initialize this in both canvas and builder,
302+
// because the events will fire in either one, depending on where the focus is
303+
// @todo we need to forward the events from canvas to builder and avoid importing this
304+
// in both places
305+
initCopyPaste(abortController);
306+
302307
const unsubscribe = $loadingState.subscribe((loadingState) => {
303308
setLoadingState(loadingState);
304309
// We need to stop updating it once it's ready in case in the future it changes again.
305310
if (loadingState.state === "ready") {
306311
unsubscribe();
307312
}
308313
});
309-
return unsubscribe;
314+
return () => {
315+
unsubscribe();
316+
abortController.abort();
317+
};
310318
}, []);
311319

312320
const canvasUrl = getCanvasUrl();

apps/builder/app/canvas/canvas.tsx

Lines changed: 41 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useMemo, useEffect, useState, useLayoutEffect } from "react";
1+
import { useMemo, useEffect, useState, useLayoutEffect, useRef } from "react";
22
import { ErrorBoundary, type FallbackProps } from "react-error-boundary";
33
import { useStore } from "@nanostores/react";
44
import type { Instances } from "@webstudio-is/sdk";
@@ -28,10 +28,10 @@ import {
2828
useCanvasStore,
2929
} from "~/shared/sync";
3030
import {
31-
useManageDesignModeStyles,
3231
GlobalStyles,
3332
subscribeStyles,
3433
mountStyles,
34+
manageDesignModeStyles,
3535
} from "./shared/styles";
3636
import {
3737
WebstudioComponentCanvas,
@@ -48,23 +48,24 @@ import {
4848
$isPreviewMode,
4949
} from "~/shared/nano-states";
5050
import { useDragAndDrop } from "./shared/use-drag-drop";
51-
import { useCopyPaste } from "~/shared/copy-paste";
51+
import { initCopyPaste } from "~/shared/copy-paste";
5252
import { setDataCollapsed, subscribeCollapsedToPubSub } from "./collapsed";
5353
import { useWindowResizeDebounced } from "~/shared/dom-hooks";
5454
import { subscribeInstanceSelection } from "./instance-selection";
5555
import { subscribeInstanceHovering } from "./instance-hovering";
5656
import { useHashLinkSync } from "~/shared/pages";
5757
import { useMount } from "~/shared/hook-utils/use-mount";
58-
import { useSelectedInstance } from "./use-selected-instance";
5958
import { subscribeInterceptedEvents } from "./interceptor";
6059
import type { ImageLoader } from "@webstudio-is/image";
6160
import { subscribeCommands } from "~/canvas/shared/commands";
6261
import { updateCollaborativeInstanceRect } from "./collaborative-instance";
6362
import { $params } from "./stores";
64-
import { useScrollNewInstanceIntoView } from "./shared/use-scroll-new-instance-into-view";
6563
import { subscribeInspectorEdits } from "./inspector-edits";
6664
import { initCanvasApi } from "~/shared/canvas-api";
6765
import { subscribeFontLoadingDone } from "./shared/font-weight-support";
66+
import { useDebounceEffect } from "~/shared/hook-utils/use-debounce-effect";
67+
import { subscribeSelected } from "./instance-selected";
68+
import { subscribeScrollNewInstanceIntoView } from "./shared/scroll-new-instance-into-view";
6869

6970
registerContainers();
7071

@@ -130,21 +131,43 @@ const useElementsTree = (
130131
};
131132

132133
const DesignMode = () => {
133-
useManageDesignModeStyles();
134+
const debounceEffect = useDebounceEffect();
135+
const ref = useRef<Instances>();
136+
134137
useDragAndDrop();
135-
// We need to initialize this in both canvas and builder,
136-
// because the events will fire in either one, depending on where the focus is
137-
// @todo we need to forward the events from canvas to builder and avoid importing this
138-
// in both places
139-
useCopyPaste();
140138

141-
useScrollNewInstanceIntoView();
142-
useSelectedInstance();
143-
useEffect(updateCollaborativeInstanceRect, []);
144-
useEffect(subscribeInstanceSelection, []);
145-
useEffect(subscribeInstanceHovering, []);
146-
useEffect(subscribeInspectorEdits, []);
147-
useEffect(subscribeFontLoadingDone, []);
139+
useEffect(() => {
140+
const abortController = new AbortController();
141+
subscribeScrollNewInstanceIntoView(
142+
debounceEffect,
143+
ref,
144+
abortController.signal
145+
);
146+
const unsubscribeSelected = subscribeSelected(debounceEffect);
147+
return () => {
148+
unsubscribeSelected();
149+
abortController.abort();
150+
};
151+
}, [debounceEffect]);
152+
153+
useEffect(() => {
154+
const abortController = new AbortController();
155+
const options = { signal: abortController.signal };
156+
// We need to initialize this in both canvas and builder,
157+
// because the events will fire in either one, depending on where the focus is
158+
// @todo we need to forward the events from canvas to builder and avoid importing this
159+
// in both places
160+
initCopyPaste(options);
161+
manageDesignModeStyles(options);
162+
updateCollaborativeInstanceRect(options);
163+
subscribeInstanceSelection(options);
164+
subscribeInstanceHovering(options);
165+
subscribeInspectorEdits(options);
166+
subscribeFontLoadingDone(options);
167+
return () => {
168+
abortController.abort();
169+
};
170+
}, []);
148171
return null;
149172
};
150173

apps/builder/app/canvas/collaborative-instance.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,11 @@ import {
77
$collaborativeInstanceRect,
88
} from "~/shared/nano-states";
99

10-
export const updateCollaborativeInstanceRect = () => {
10+
export const updateCollaborativeInstanceRect = ({
11+
signal,
12+
}: {
13+
signal: AbortSignal;
14+
}) => {
1115
let frameHandler: number = -1;
1216
let elements: HTMLElement[] = [];
1317

@@ -43,8 +47,8 @@ export const updateCollaborativeInstanceRect = () => {
4347
}
4448
});
4549

46-
return () => {
50+
signal.addEventListener("abort", () => {
4751
unsubscribe();
4852
cancelAnimationFrame(frameHandler);
49-
};
53+
});
5054
};
Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,13 @@
11
import { canvasApi } from "~/shared/canvas-api";
22
import { $props } from "~/shared/nano-states";
33

4-
export const subscribeInspectorEdits = () => {
5-
/**
6-
* Prevents Radix from stealing focus when the content inside a dialog changes.
7-
* (Radix focus scope uses a MutationObserver)
8-
*/
4+
export const subscribeInspectorEdits = ({
5+
signal,
6+
}: {
7+
signal: AbortSignal;
8+
}) => {
9+
// Prevents Radix from stealing focus when the content inside a dialog changes.
10+
// (Radix focus scope uses a MutationObserver)
911
const unsubscribeProps = $props.listen(canvasApi.setInert);
10-
11-
return () => {
12-
unsubscribeProps();
13-
};
12+
signal.addEventListener("abort", unsubscribeProps);
1413
};

apps/builder/app/canvas/instance-hovering.ts

Lines changed: 39 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,11 @@ import type { InstanceSelector } from "~/shared/tree-utils";
1111

1212
type TimeoutId = undefined | ReturnType<typeof setTimeout>;
1313

14-
export const subscribeInstanceHovering = () => {
14+
export const subscribeInstanceHovering = ({
15+
signal,
16+
}: {
17+
signal: AbortSignal;
18+
}) => {
1519
let hoveredElement: undefined | Element = undefined;
1620
let isScrolling = false;
1721

@@ -24,35 +28,41 @@ export const subscribeInstanceHovering = () => {
2428

2529
let mouseOutTimeoutId: TimeoutId = undefined;
2630

27-
const handleMouseOver = (event: MouseEvent) => {
28-
if (event.target instanceof Element) {
29-
const element = event.target.closest(`[${idAttribute}]`) ?? undefined;
30-
if (element !== undefined) {
31-
clearTimeout(mouseOutTimeoutId);
32-
// store hovered element locally to update outline when scroll ends
33-
hoveredElement = element;
34-
updateHoveredInstance(element);
31+
const eventOptions = { passive: true, signal };
32+
33+
window.addEventListener(
34+
"mouseover",
35+
(event: MouseEvent) => {
36+
if (event.target instanceof Element) {
37+
const element = event.target.closest(`[${idAttribute}]`) ?? undefined;
38+
if (element !== undefined) {
39+
clearTimeout(mouseOutTimeoutId);
40+
// store hovered element locally to update outline when scroll ends
41+
hoveredElement = element;
42+
updateHoveredInstance(element);
43+
}
3544
}
36-
}
37-
};
38-
39-
const handleMouseOut = () => {
40-
mouseOutTimeoutId = setTimeout(() => {
41-
hoveredElement = undefined;
42-
$hoveredInstanceSelector.set(undefined);
43-
$hoveredInstanceOutline.set(undefined);
44-
}, 100);
45+
},
46+
eventOptions
47+
);
4548

46-
// Fixes the bug, that new hover occures during timeout
47-
const unsubscribe = $hoveredInstanceSelector.listen(() => {
48-
clearTimeout(mouseOutTimeoutId);
49-
unsubscribe();
50-
});
51-
};
49+
window.addEventListener(
50+
"mouseout",
51+
() => {
52+
mouseOutTimeoutId = setTimeout(() => {
53+
hoveredElement = undefined;
54+
$hoveredInstanceSelector.set(undefined);
55+
$hoveredInstanceOutline.set(undefined);
56+
}, 100);
5257

53-
const eventOptions = { passive: true };
54-
window.addEventListener("mouseover", handleMouseOver, eventOptions);
55-
window.addEventListener("mouseout", handleMouseOut, eventOptions);
58+
// Fixes the bug, that new hover occures during timeout
59+
const unsubscribe = $hoveredInstanceSelector.listen(() => {
60+
clearTimeout(mouseOutTimeoutId);
61+
unsubscribe();
62+
});
63+
},
64+
eventOptions
65+
);
5666

5767
const updateHoveredRect = (
5868
elements: Element[],
@@ -114,11 +124,9 @@ export const subscribeInstanceHovering = () => {
114124
}
115125
);
116126

117-
return () => {
118-
window.removeEventListener("mouseover", handleMouseOver);
119-
window.removeEventListener("mouseout", handleMouseOut);
127+
signal.addEventListener("abort", () => {
120128
unsubscribeScrollState();
121129
clearTimeout(mouseOutTimeoutId);
122130
unsubscribeHoveredInstanceId();
123-
};
131+
});
124132
};

apps/builder/app/canvas/instance-selection.ts

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,11 @@ import {
88
import { $textEditingInstanceSelector } from "~/shared/nano-states";
99
import { emitCommand } from "./shared/commands";
1010

11-
export const subscribeInstanceSelection = () => {
11+
export const subscribeInstanceSelection = ({
12+
signal,
13+
}: {
14+
signal: AbortSignal;
15+
}) => {
1216
let pointerDownElement: undefined | Element = undefined;
1317
let lastPointerUpTime = 0;
1418
let clickCount = 1;
@@ -78,11 +82,6 @@ export const subscribeInstanceSelection = () => {
7882
}
7983
};
8084

81-
addEventListener("pointerdown", handlePointerDown, { passive: true });
82-
addEventListener("pointerup", handlePointerUp, { passive: true });
83-
84-
return () => {
85-
removeEventListener("pointerdown", handlePointerDown);
86-
removeEventListener("pointerup", handlePointerUp);
87-
};
85+
addEventListener("pointerdown", handlePointerDown, { passive: true, signal });
86+
addEventListener("pointerup", handlePointerUp, { passive: true, signal });
8887
};

apps/builder/app/canvas/shared/font-weight-support.ts

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -81,9 +81,11 @@ const testFontWeights = (fontFamily: string) => {
8181
return supportedWeights.sort();
8282
};
8383

84-
export const subscribeFontLoadingDone = () => {
85-
// @todo move it to the call-site
86-
const abortController = new AbortController();
84+
export const subscribeFontLoadingDone = ({
85+
signal,
86+
}: {
87+
signal: AbortSignal;
88+
}) => {
8789
document.fonts.addEventListener(
8890
"loadingdone",
8991
() => {
@@ -95,12 +97,8 @@ export const subscribeFontLoadingDone = () => {
9597
}
9698
$detectedFontsWeights.set(cache);
9799
},
98-
{ signal: abortController.signal }
100+
{ signal }
99101
);
100-
101-
return () => {
102-
abortController.abort();
103-
};
104102
};
105103

106104
export const detectSupportedFontWeights = (stack: string) => {

apps/builder/app/canvas/shared/scroll-new-instance-into-view.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,10 @@ import { $instances } from "~/shared/nano-states";
77
*/
88
export const subscribeScrollNewInstanceIntoView = (
99
debounceEffect: (callback: () => void) => void,
10-
previousInstances: { current?: Instances }
10+
previousInstances: { current?: Instances },
11+
signal: AbortSignal
1112
) => {
12-
return $instances.subscribe((instances) => {
13+
const unsubscribe = $instances.subscribe((instances) => {
1314
if (previousInstances.current === undefined) {
1415
previousInstances.current = instances;
1516
return;
@@ -36,4 +37,5 @@ export const subscribeScrollNewInstanceIntoView = (
3637
});
3738
});
3839
});
40+
signal.addEventListener("abort", unsubscribe);
3941
};

apps/builder/app/canvas/shared/styles.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useEffect, useLayoutEffect } from "react";
1+
import { useLayoutEffect } from "react";
22
import { computed } from "nanostores";
33
import { useStore } from "@nanostores/react";
44
import {
@@ -307,10 +307,15 @@ export const subscribeStyles = () => {
307307
};
308308
};
309309

310-
export const useManageDesignModeStyles = () => {
311-
useEffect(subscribeStateStyles, []);
312-
useEffect(subscribeEphemeralStyle, []);
313-
useEffect(subscribePreviewMode, []);
310+
export const manageDesignModeStyles = ({ signal }: { signal: AbortSignal }) => {
311+
const unsubscribeStateStyles = subscribeStateStyles();
312+
const unsubscribeEphemeralStyle = subscribeEphemeralStyle();
313+
const unsubscribePreviewMode = subscribePreviewMode();
314+
signal.addEventListener("abort", () => {
315+
unsubscribeStateStyles();
316+
unsubscribeEphemeralStyle();
317+
unsubscribePreviewMode();
318+
});
314319
};
315320

316321
export const GlobalStyles = ({ params }: { params: Params }) => {

0 commit comments

Comments
 (0)