From 72c075e5fb90d7dfdbfaba6d0d17b0212552ebc0 Mon Sep 17 00:00:00 2001 From: peterkogo Date: Thu, 12 Dec 2024 16:14:29 +0100 Subject: [PATCH] fixed useConnection hook, made derivedWarnings more specific, fixed updating when internals change --- .../examples/add-node-on-drop/Flow.svelte | 18 ++- .../routes/examples/color-mode/+page.svelte | 9 +- .../custom-connection-line/+page.svelte | 11 +- .../ConnectionLine.svelte | 10 +- .../routes/examples/customnode/+page.svelte | 24 ++-- .../examples/customnode/CustomNode.svelte | 10 +- .../src/routes/examples/dagre/+page.svelte | 117 +++++++++--------- .../ConnectionLine/ConnectionLine.svelte | 2 - .../container/SvelteFlow/SvelteFlow.svelte | 2 +- .../src/lib/hooks/derivedWarning.svelte.ts | 16 ++- .../src/lib/hooks/useConnection.svelte.ts | 18 +++ .../svelte/src/lib/hooks/useConnection.ts | 16 --- .../src/lib/hooks/useHandleEdgeSelect.ts | 24 ++-- packages/svelte/src/lib/index.ts | 2 +- .../src/lib/store/initial-store.svelte.ts | 20 ++- packages/system/src/utils/store.ts | 31 +++-- 16 files changed, 176 insertions(+), 154 deletions(-) create mode 100644 packages/svelte/src/lib/hooks/useConnection.svelte.ts delete mode 100644 packages/svelte/src/lib/hooks/useConnection.ts diff --git a/examples/svelte/src/routes/examples/add-node-on-drop/Flow.svelte b/examples/svelte/src/routes/examples/add-node-on-drop/Flow.svelte index 25a7c2ad..f0bd7a74 100644 --- a/examples/svelte/src/routes/examples/add-node-on-drop/Flow.svelte +++ b/examples/svelte/src/routes/examples/add-node-on-drop/Flow.svelte @@ -1,5 +1,4 @@ @@ -73,8 +69,8 @@
{ diff --git a/examples/svelte/src/routes/examples/color-mode/+page.svelte b/examples/svelte/src/routes/examples/color-mode/+page.svelte index 13f201f5..e23c4d25 100644 --- a/examples/svelte/src/routes/examples/color-mode/+page.svelte +++ b/examples/svelte/src/routes/examples/color-mode/+page.svelte @@ -1,5 +1,4 @@ - + diff --git a/examples/svelte/src/routes/examples/custom-connection-line/+page.svelte b/examples/svelte/src/routes/examples/custom-connection-line/+page.svelte index c2b1951f..ea961d65 100644 --- a/examples/svelte/src/routes/examples/custom-connection-line/+page.svelte +++ b/examples/svelte/src/routes/examples/custom-connection-line/+page.svelte @@ -1,7 +1,6 @@
- + {#snippet connectionLine()} - - {/snippet} + + {/snippet}
diff --git a/examples/svelte/src/routes/examples/custom-connection-line/ConnectionLine.svelte b/examples/svelte/src/routes/examples/custom-connection-line/ConnectionLine.svelte index f670fbc5..7782903b 100644 --- a/examples/svelte/src/routes/examples/custom-connection-line/ConnectionLine.svelte +++ b/examples/svelte/src/routes/examples/custom-connection-line/ConnectionLine.svelte @@ -1,11 +1,11 @@ -{#if $connection.inProgress} - +{#if connection.inProgress} + {/if} diff --git a/examples/svelte/src/routes/examples/customnode/+page.svelte b/examples/svelte/src/routes/examples/customnode/+page.svelte index f89210c8..5acfe767 100644 --- a/examples/svelte/src/routes/examples/customnode/+page.svelte +++ b/examples/svelte/src/routes/examples/customnode/+page.svelte @@ -1,5 +1,13 @@ + + diff --git a/examples/svelte/src/routes/examples/customnode/CustomNode.svelte b/examples/svelte/src/routes/examples/customnode/CustomNode.svelte index dc3abb13..6bc71e2e 100644 --- a/examples/svelte/src/routes/examples/customnode/CustomNode.svelte +++ b/examples/svelte/src/routes/examples/customnode/CustomNode.svelte @@ -1,8 +1,8 @@ @@ -10,13 +10,13 @@
- Custom Color Picker Node: {$colorStore} + Custom Color Picker Node: {bg.color}
colorStore.set(evt.currentTarget.value)} - value={$colorStore} + oninput={(evt) => (bg.color = evt.currentTarget.value)} + value={bg.color} /> diff --git a/examples/svelte/src/routes/examples/dagre/+page.svelte b/examples/svelte/src/routes/examples/dagre/+page.svelte index b5e1e897..88470acc 100644 --- a/examples/svelte/src/routes/examples/dagre/+page.svelte +++ b/examples/svelte/src/routes/examples/dagre/+page.svelte @@ -1,79 +1,78 @@
- - - - - - - + + + + + + +
diff --git a/packages/svelte/src/lib/components/ConnectionLine/ConnectionLine.svelte b/packages/svelte/src/lib/components/ConnectionLine/ConnectionLine.svelte index 221b9eab..490088d6 100644 --- a/packages/svelte/src/lib/components/ConnectionLine/ConnectionLine.svelte +++ b/packages/svelte/src/lib/components/ConnectionLine/ConnectionLine.svelte @@ -23,8 +23,6 @@ connectionLine?: Snippet; } = $props(); - // $inspect(store.connection); - let path = $derived.by(() => { if (!store.connection.inProgress) { return ''; diff --git a/packages/svelte/src/lib/container/SvelteFlow/SvelteFlow.svelte b/packages/svelte/src/lib/container/SvelteFlow/SvelteFlow.svelte index 2c4cb3f1..53d13a56 100644 --- a/packages/svelte/src/lib/container/SvelteFlow/SvelteFlow.svelte +++ b/packages/svelte/src/lib/container/SvelteFlow/SvelteFlow.svelte @@ -99,7 +99,7 @@ // Set store for provider context const providerContext = getContext(key); - if (providerContext) { + if (providerContext && providerContext.setStore) { providerContext.setStore(store); } diff --git a/packages/svelte/src/lib/hooks/derivedWarning.svelte.ts b/packages/svelte/src/lib/hooks/derivedWarning.svelte.ts index 3a1a4b62..ce7226c9 100644 --- a/packages/svelte/src/lib/hooks/derivedWarning.svelte.ts +++ b/packages/svelte/src/lib/hooks/derivedWarning.svelte.ts @@ -2,7 +2,14 @@ import { key } from '$lib/store'; import type { StoreContext } from '$lib/store/types'; import { getContext } from 'svelte'; -export function derivedWarning(functionName: string) { +/** + * Warns the user that they should use $derived() when calling a hook. + * This is not neccessarry when the hook is called inside a child of , + * however exceptions can be made if you don't want to return a closure. + * @param functionName - The name of the function that is being called + * @param force - If true, the warning will be shown regardless if child of + */ +export function derivedWarning(functionName: string, force?: boolean) { const storeContext = getContext(key); if (!storeContext) { @@ -11,9 +18,8 @@ export function derivedWarning(functionName: string) { ); } - if (storeContext.provider && !$effect.tracking()) { - console.warn( - `Use $derived(${functionName}()), when not calling inside a child of the component.` - ); + if ((force || storeContext.provider) && !$effect.tracking()) { + console.warn(`Use $derived(${functionName}()) to receive updates when values change.`); + console.trace(functionName); } } diff --git a/packages/svelte/src/lib/hooks/useConnection.svelte.ts b/packages/svelte/src/lib/hooks/useConnection.svelte.ts new file mode 100644 index 00000000..7032a68a --- /dev/null +++ b/packages/svelte/src/lib/hooks/useConnection.svelte.ts @@ -0,0 +1,18 @@ +import { useStore } from '$lib/store'; + +import type { ConnectionState } from '@xyflow/system'; +import { derivedWarning } from './derivedWarning.svelte'; + +/** + * Hook for receiving the current connection. + * + * @public + * @returns current connection as a readable store + */ +export function useConnection(): ConnectionState { + if (process.env.NODE_ENV === 'development') { + derivedWarning('useConnection', true); + } + + return useStore().connection; +} diff --git a/packages/svelte/src/lib/hooks/useConnection.ts b/packages/svelte/src/lib/hooks/useConnection.ts deleted file mode 100644 index 4402ab69..00000000 --- a/packages/svelte/src/lib/hooks/useConnection.ts +++ /dev/null @@ -1,16 +0,0 @@ -import type { Readable } from 'svelte/store'; -import { useStore } from '$lib/store'; - -import type { ConnectionState } from '@xyflow/system'; - -/** - * Hook for receiving the current connection. - * - * @public - * @returns current connection as a readable store - */ -export function useConnection(): Readable { - const { connection } = useStore(); - - return connection; -} diff --git a/packages/svelte/src/lib/hooks/useHandleEdgeSelect.ts b/packages/svelte/src/lib/hooks/useHandleEdgeSelect.ts index 5e6ba5d3..fcb0bd09 100644 --- a/packages/svelte/src/lib/hooks/useHandleEdgeSelect.ts +++ b/packages/svelte/src/lib/hooks/useHandleEdgeSelect.ts @@ -4,18 +4,10 @@ import { errorMessages } from '@xyflow/system'; import { useStore } from '$lib/store'; export function useHandleEdgeSelect() { - const { - edgeLookup, - selectionRect, - selectionRectMode, - multiselectionKeyPressed, - addSelectedEdges, - unselectNodesAndEdges, - elementsSelectable - } = useStore(); + const store = useStore(); return (id: string) => { - const edge = get(edgeLookup).get(id); + const edge = store.edgeLookup.get(id); if (!edge) { console.warn('012', errorMessages['error012'](id)); @@ -23,16 +15,16 @@ export function useHandleEdgeSelect() { } const selectable = - edge.selectable || (get(elementsSelectable) && typeof edge.selectable === 'undefined'); + edge.selectable || (store.elementsSelectable && typeof edge.selectable === 'undefined'); if (selectable) { - selectionRect.set(null); - selectionRectMode.set(null); + store.selectionRect = null; + store.selectionRectMode = null; if (!edge.selected) { - addSelectedEdges([id]); - } else if (edge.selected && get(multiselectionKeyPressed)) { - unselectNodesAndEdges({ nodes: [], edges: [edge] }); + store.addSelectedEdges([id]); + } else if (edge.selected && store.multiselectionKeyPressed) { + store.unselectNodesAndEdges({ nodes: [], edges: [edge] }); } } }; diff --git a/packages/svelte/src/lib/index.ts b/packages/svelte/src/lib/index.ts index 2cf48b73..8ac9da57 100644 --- a/packages/svelte/src/lib/index.ts +++ b/packages/svelte/src/lib/index.ts @@ -33,7 +33,7 @@ export * from '$lib/utils'; //hooks export * from '$lib/hooks/useSvelteFlow'; export * from '$lib/hooks/useUpdateNodeInternals'; -export * from '$lib/hooks/useConnection'; +export * from '$lib/hooks/useConnection.svelte'; export * from '$lib/hooks/useNodesEdges'; export * from '$lib/hooks/useHandleConnections'; export * from '$lib/hooks/useNodesData'; diff --git a/packages/svelte/src/lib/store/initial-store.svelte.ts b/packages/svelte/src/lib/store/initial-store.svelte.ts index a06aa6bd..520c1ae4 100644 --- a/packages/svelte/src/lib/store/initial-store.svelte.ts +++ b/packages/svelte/src/lib/store/initial-store.svelte.ts @@ -72,7 +72,7 @@ export const initialEdgeTypes = { export const getInitialStore = (signals: StoreSignals) => { // We use a class here, because Svelte adds getters & setter for us. - // Inline classes have some performance implications but we just call it once. + // Inline classes have some performance implications but we just call it once (max twice). class SvelteFlowStore { get nodes() { return signals.nodes; @@ -223,6 +223,11 @@ export const getInitialStore = (signals: StoreSignals) => { }); this.viewport = getViewportForBounds(bounds, this.width, this.height, 0.5, 2, 0.1); } + + if (process.env.NODE_ENV === 'development') { + warnIfDeeplyReactive(signals.nodes, 'nodes'); + warnIfDeeplyReactive(signals.edges, 'edges'); + } } resetStoreValues() { @@ -231,3 +236,16 @@ export const getInitialStore = (signals: StoreSignals) => { } return new SvelteFlowStore(); }; + +// Only way to check if an object is a proxy +// is to see if is failes to perform a structured clone +// TODO: is $state.raw really nessessary? +function warnIfDeeplyReactive(array: unknown[] | undefined, name: string) { + try { + if (array && array.length > 0) { + structuredClone(array[0]); + } + } catch { + console.warn(`Use $state.raw for ${name} to prevent performance issues.`); + } +} diff --git a/packages/system/src/utils/store.ts b/packages/system/src/utils/store.ts index 82494343..d62fb534 100644 --- a/packages/system/src/utils/store.ts +++ b/packages/system/src/utils/store.ts @@ -337,10 +337,13 @@ export function updateNodeInternals( } if (node.hidden) { - node.internals = { - ...node.internals, - handleBounds: undefined, - }; + nodeLookup.set(node.id, { + ...node, + internals: { + ...node.internals, + handleBounds: undefined, + }, + }); updatedInternals = true; } else { const dimensions = getDimensions(update.nodeElement); @@ -362,15 +365,19 @@ export function updateNodeInternals( positionAbsolute = clampPosition(positionAbsolute, extent, dimensions); } - node.measured = dimensions; - node.internals = { - ...node.internals, - positionAbsolute, - handleBounds: { - source: getHandleBounds('source', update.nodeElement, nodeBounds, zoom, node.id), - target: getHandleBounds('target', update.nodeElement, nodeBounds, zoom, node.id), + nodeLookup.set(node.id, { + ...node, + measured: dimensions, + internals: { + ...node.internals, + positionAbsolute, + handleBounds: { + source: getHandleBounds('source', update.nodeElement, nodeBounds, zoom, node.id), + target: getHandleBounds('target', update.nodeElement, nodeBounds, zoom, node.id), + }, }, - }; + }); + if (node.parentId) { updateChildNode(node, nodeLookup, parentLookup, { nodeOrigin }); }