Refactor watch sources for reliability

This commit changes `WatchSource` signatures into `Readonly<Ref>`s.

It provides two important benefits:

1. Eliminates the possibility of `undefined` states, that's result of
   using `WatchSource`s. This previously required additional null checks.
   By using `Readonly<Ref>`, the state handling becomes simpler and less
   susceptible to null errors.
2. Optimizes performance by using references:
   - Avoids the reactive layer of `computed` references when not needed.
   - The `watch` syntax, such as `watch(() => ref.value)`, can introduce
     side effects. For example, it does not account for `triggerRef` in
     scenarios where the value remains unchanged, preventing the watcher
     from running (vuejs/core#9579).
This commit is contained in:
undergroundwires
2023-11-11 13:55:21 +01:00
parent 58cd551a30
commit 7ab16ecccb
25 changed files with 190 additions and 217 deletions

View File

@@ -17,7 +17,7 @@
</template>
<script lang="ts">
import { defineComponent } from 'vue';
import { defineComponent, toRef } from 'vue';
import { injectKey } from '@/presentation/injectionSymbols';
import TreeView from './TreeView/TreeView.vue';
import NodeContent from './NodeContent/NodeContent.vue';
@@ -42,7 +42,7 @@ export default defineComponent({
const useUserCollectionStateHook = injectKey((keys) => keys.useUserSelectionState);
const { selectedScriptNodeIds } = useSelectedScriptNodeIds(useUserCollectionStateHook);
const { latestFilterEvent } = useTreeViewFilterEvent();
const { treeViewInputNodes } = useTreeViewNodeInput(() => props.categoryId);
const { treeViewInputNodes } = useTreeViewNodeInput(toRef(props, 'categoryId'));
const { updateNodeSelection } = useCollectionSelectionStateUpdater(useUserCollectionStateHook);
function handleNodeChangedEvent(event: TreeNodeStateChangedEmittedEvent) {

View File

@@ -1,5 +1,5 @@
<template>
<div class="wrapper" v-if="currentNode">
<div class="wrapper">
<div
class="expansible-node"
@click="toggleCheck"
@@ -46,15 +46,14 @@
</template>
<script lang="ts">
import {
defineComponent, computed, PropType,
} from 'vue';
import { defineComponent, computed, toRef } from 'vue';
import { TreeRoot } from '../TreeRoot/TreeRoot';
import { useCurrentTreeNodes } from '../UseCurrentTreeNodes';
import { NodeRenderingStrategy } from '../Rendering/Scheduling/NodeRenderingStrategy';
import { useNodeState } from './UseNodeState';
import { TreeNode } from './TreeNode';
import LeafTreeNode from './LeafTreeNode.vue';
import type { PropType } from 'vue';
export default defineComponent({
name: 'HierarchicalTreeNode', // Needed due to recursion
@@ -76,33 +75,32 @@ export default defineComponent({
},
},
setup(props) {
const { nodes } = useCurrentTreeNodes(() => props.treeRoot);
const currentNode = computed<TreeNode | undefined>(
() => nodes.value?.getNodeById(props.nodeId),
const { nodes } = useCurrentTreeNodes(toRef(props, 'treeRoot'));
const currentNode = computed<TreeNode>(
() => nodes.value.getNodeById(props.nodeId),
);
const { state } = useNodeState(() => currentNode.value);
const expanded = computed<boolean>(() => state.value?.isExpanded ?? false);
const { state } = useNodeState(currentNode);
const expanded = computed<boolean>(() => state.value.isExpanded);
const renderedNodeIds = computed<readonly string[]>(
() => currentNode.value
?.hierarchy
.hierarchy
.children
.filter((child) => props.renderingStrategy.shouldRender(child))
.map((child) => child.id)
?? [],
.map((child) => child.id),
);
function toggleExpand() {
currentNode.value?.state.toggleExpand();
currentNode.value.state.toggleExpand();
}
function toggleCheck() {
currentNode.value?.state.toggleCheck();
currentNode.value.state.toggleCheck();
}
const hasChildren = computed<boolean>(
() => currentNode.value?.hierarchy.isBranchNode,
() => currentNode.value.hierarchy.isBranchNode,
);
return {

View File

@@ -1,6 +1,5 @@
<template>
<li
v-if="currentNode"
class="wrapper"
@click.stop="toggleCheckState"
>
@@ -31,15 +30,14 @@
</template>
<script lang="ts">
import {
defineComponent, computed, PropType,
} from 'vue';
import { defineComponent, computed, toRef } from 'vue';
import { TreeRoot } from '../TreeRoot/TreeRoot';
import { useCurrentTreeNodes } from '../UseCurrentTreeNodes';
import { useNodeState } from './UseNodeState';
import { useKeyboardInteractionState } from './UseKeyboardInteractionState';
import { TreeNode } from './TreeNode';
import { TreeNodeCheckState } from './State/CheckState';
import type { PropType } from 'vue';
export default defineComponent({
props: {
@@ -54,16 +52,16 @@ export default defineComponent({
},
setup(props) {
const { isKeyboardBeingUsed } = useKeyboardInteractionState();
const { nodes } = useCurrentTreeNodes(() => props.treeRoot);
const currentNode = computed<TreeNode | undefined>(
() => nodes.value?.getNodeById(props.nodeId),
const { nodes } = useCurrentTreeNodes(toRef(props, 'treeRoot'));
const currentNode = computed<TreeNode>(
() => nodes.value.getNodeById(props.nodeId),
);
const { state } = useNodeState(() => currentNode.value);
const { state } = useNodeState(currentNode);
const hasFocus = computed<boolean>(() => state.value?.isFocused ?? false);
const checked = computed<boolean>(() => state.value?.checkState === TreeNodeCheckState.Checked);
const hasFocus = computed<boolean>(() => state.value.isFocused);
const checked = computed<boolean>(() => state.value.checkState === TreeNodeCheckState.Checked);
const indeterminate = computed<boolean>(
() => state.value?.checkState === TreeNodeCheckState.Indeterminate,
() => state.value.checkState === TreeNodeCheckState.Indeterminate,
);
const hasKeyboardFocus = computed<boolean>(() => {
@@ -74,14 +72,11 @@ export default defineComponent({
});
const onNodeFocus = () => {
if (!currentNode.value) {
return;
}
props.treeRoot.focus.setSingleFocus(currentNode.value);
};
function toggleCheckState() {
currentNode.value?.state.toggleCheck();
currentNode.value.state.toggleCheck();
}
return {

View File

@@ -1,21 +1,18 @@
import {
WatchSource, shallowRef, watch,
type Ref, shallowRef, watch, shallowReadonly,
} from 'vue';
import { injectKey } from '@/presentation/injectionSymbols';
import { ReadOnlyTreeNode } from './TreeNode';
import { TreeNodeStateDescriptor } from './State/StateDescriptor';
export function useNodeState(
nodeWatcher: WatchSource<ReadOnlyTreeNode | undefined>,
nodeRef: Readonly<Ref<ReadOnlyTreeNode>>,
) {
const { events } = injectKey((keys) => keys.useAutoUnsubscribedEvents);
const state = shallowRef<TreeNodeStateDescriptor>();
const state = shallowRef<TreeNodeStateDescriptor>(nodeRef.value.state.current);
watch(nodeWatcher, (node: ReadOnlyTreeNode) => {
if (!node) {
return;
}
watch(nodeRef, (node: ReadOnlyTreeNode) => {
state.value = node.state.current;
events.unsubscribeAllAndRegister([
node.state.changed.on((change) => {
@@ -25,6 +22,6 @@ export function useNodeState(
}, { immediate: true });
return {
state,
state: shallowReadonly(state),
};
}

View File

@@ -1,5 +1,5 @@
import {
WatchSource, shallowRef, triggerRef, watch,
type Ref, shallowRef, triggerRef, watch,
} from 'vue';
import { ReadOnlyTreeNode } from '../Node/TreeNode';
import { useNodeStateChangeAggregator } from '../UseNodeStateChangeAggregator';
@@ -15,7 +15,7 @@ import { CollapsedParentOrderer } from './Ordering/CollapsedParentOrderer';
* Renders tree nodes gradually to prevent UI freeze when loading large amounts of nodes.
*/
export function useGradualNodeRendering(
treeWatcher: WatchSource<TreeRoot>,
treeRootRef: Readonly<Ref<TreeRoot>>,
useChangeAggregator = useNodeStateChangeAggregator,
useTreeNodes = useCurrentTreeNodes,
scheduler: DelayScheduler = new TimeoutDelayScheduler(),
@@ -28,8 +28,8 @@ export function useGradualNodeRendering(
let isRenderingInProgress = false;
const renderingDelayInMs = 50;
const { onNodeStateChange } = useChangeAggregator(treeWatcher);
const { nodes } = useTreeNodes(treeWatcher);
const { onNodeStateChange } = useChangeAggregator(treeRootRef);
const { nodes } = useTreeNodes(treeRootRef);
function updateNodeRenderQueue(node: ReadOnlyTreeNode, isVisible: boolean) {
if (isVisible
@@ -48,7 +48,7 @@ export function useGradualNodeRendering(
}
}
watch(() => nodes.value, (newNodes) => {
watch(nodes, (newNodes) => {
nodesToRender.clear();
nodesBeingRendered.value.clear();
if (!newNodes || newNodes.flattenedNodes.length === 0) {

View File

@@ -18,12 +18,13 @@
<script lang="ts">
import {
defineComponent, computed, PropType,
defineComponent, computed, toRef,
} from 'vue';
import HierarchicalTreeNode from '../Node/HierarchicalTreeNode.vue';
import { useCurrentTreeNodes } from '../UseCurrentTreeNodes';
import { NodeRenderingStrategy } from '../Rendering/Scheduling/NodeRenderingStrategy';
import { TreeRoot } from './TreeRoot';
import type { PropType } from 'vue';
export default defineComponent({
components: {
@@ -40,7 +41,7 @@ export default defineComponent({
},
},
setup(props) {
const { nodes } = useCurrentTreeNodes(() => props.treeRoot);
const { nodes } = useCurrentTreeNodes(toRef(props, 'treeRoot'));
const renderedNodeIds = computed<string[]>(() => {
return nodes

View File

@@ -14,7 +14,7 @@
<script lang="ts">
import {
defineComponent, onMounted, watch,
shallowRef, PropType,
shallowRef, toRef, shallowReadonly,
} from 'vue';
import { TreeRootManager } from './TreeRoot/TreeRootManager';
import TreeRoot from './TreeRoot/TreeRoot.vue';
@@ -28,6 +28,7 @@ import { TreeNodeStateChangedEmittedEvent } from './Bindings/TreeNodeStateChange
import { useAutoUpdateParentCheckState } from './UseAutoUpdateParentCheckState';
import { useAutoUpdateChildrenCheckState } from './UseAutoUpdateChildrenCheckState';
import { useGradualNodeRendering } from './Rendering/UseGradualNodeRendering';
import type { PropType } from 'vue';
export default defineComponent({
components: {
@@ -57,17 +58,16 @@ export default defineComponent({
const tree = new TreeRootManager();
useTreeKeyboardNavigation(tree, treeContainerElement);
useTreeQueryFilter(
() => props.latestFilterEvent,
() => tree,
);
useLeafNodeCheckedStateUpdater(() => tree, () => props.selectedLeafNodeIds);
useAutoUpdateParentCheckState(() => tree);
useAutoUpdateChildrenCheckState(() => tree);
const nodeRenderingScheduler = useGradualNodeRendering(() => tree);
const treeRef = shallowReadonly(shallowRef(tree));
const { onNodeStateChange } = useNodeStateChangeAggregator(() => tree);
useTreeKeyboardNavigation(treeRef, treeContainerElement);
useTreeQueryFilter(toRef(props, 'latestFilterEvent'), treeRef);
useLeafNodeCheckedStateUpdater(treeRef, toRef(props, 'selectedLeafNodeIds'));
useAutoUpdateParentCheckState(treeRef);
useAutoUpdateChildrenCheckState(treeRef);
const nodeRenderingScheduler = useGradualNodeRendering(treeRef);
const { onNodeStateChange } = useNodeStateChangeAggregator(treeRef);
onNodeStateChange((change) => {
emit('nodeStateChanged', {

View File

@@ -1,14 +1,14 @@
import { WatchSource } from 'vue';
import { TreeRoot } from './TreeRoot/TreeRoot';
import { useNodeStateChangeAggregator } from './UseNodeStateChangeAggregator';
import { HierarchyAccess } from './Node/Hierarchy/HierarchyAccess';
import { TreeNodeCheckState } from './Node/State/CheckState';
import type { Ref } from 'vue';
export function useAutoUpdateChildrenCheckState(
treeWatcher: WatchSource<TreeRoot>,
treeRootRef: Readonly<Ref<TreeRoot>>,
useChangeAggregator = useNodeStateChangeAggregator,
) {
const { onNodeStateChange } = useChangeAggregator(treeWatcher);
const { onNodeStateChange } = useChangeAggregator(treeRootRef);
onNodeStateChange((change) => {
if (change.newState.checkState === change.oldState?.checkState) {

View File

@@ -1,15 +1,15 @@
import { WatchSource } from 'vue';
import { TreeRoot } from './TreeRoot/TreeRoot';
import { useNodeStateChangeAggregator } from './UseNodeStateChangeAggregator';
import { HierarchyAccess } from './Node/Hierarchy/HierarchyAccess';
import { TreeNodeCheckState } from './Node/State/CheckState';
import { ReadOnlyTreeNode } from './Node/TreeNode';
import type { Ref } from 'vue';
export function useAutoUpdateParentCheckState(
treeWatcher: WatchSource<TreeRoot>,
treeRef: Readonly<Ref<TreeRoot>>,
useChangeAggregator = useNodeStateChangeAggregator,
) {
const { onNodeStateChange } = useChangeAggregator(treeWatcher);
const { onNodeStateChange } = useChangeAggregator(treeRef);
onNodeStateChange((change) => {
if (change.newState.checkState === change.oldState?.checkState) {

View File

@@ -1,18 +1,16 @@
import {
WatchSource, watch, shallowReadonly, shallowRef,
watch, shallowReadonly, shallowRef, type Ref,
} from 'vue';
import { injectKey } from '@/presentation/injectionSymbols';
import { TreeRoot } from './TreeRoot/TreeRoot';
import { QueryableNodes } from './TreeRoot/NodeCollection/Query/QueryableNodes';
export function useCurrentTreeNodes(treeWatcher: WatchSource<TreeRoot>) {
export function useCurrentTreeNodes(treeRef: Readonly<Ref<TreeRoot>>) {
const { events } = injectKey((keys) => keys.useAutoUnsubscribedEvents);
const tree = shallowRef<TreeRoot | undefined>();
const nodes = shallowRef<QueryableNodes | undefined>();
const nodes = shallowRef<QueryableNodes>(treeRef.value.collection.nodes);
watch(treeWatcher, (newTree) => {
tree.value = newTree;
watch(treeRef, (newTree) => {
nodes.value = newTree.collection.nodes;
events.unsubscribeAllAndRegister([
newTree.collection.nodesUpdated.on((newNodes) => {

View File

@@ -1,4 +1,4 @@
import { WatchSource, watch } from 'vue';
import { type Ref, watch } from 'vue';
import { TreeRoot } from './TreeRoot/TreeRoot';
import { TreeNode } from './Node/TreeNode';
import { QueryableNodes } from './TreeRoot/NodeCollection/Query/QueryableNodes';
@@ -6,13 +6,13 @@ import { useCurrentTreeNodes } from './UseCurrentTreeNodes';
import { TreeNodeCheckState } from './Node/State/CheckState';
export function useLeafNodeCheckedStateUpdater(
treeWatcher: WatchSource<TreeRoot>,
leafNodeIdsWatcher: WatchSource<readonly string[]>,
treeRootRef: Readonly<Ref<TreeRoot>>,
leafNodeIdsRef: Readonly<Ref<readonly string[]>>,
) {
const { nodes } = useCurrentTreeNodes(treeWatcher);
const { nodes } = useCurrentTreeNodes(treeRootRef);
watch(
[leafNodeIdsWatcher, () => nodes.value],
[leafNodeIdsRef, nodes],
([nodeIds, actualNodes]) => {
updateNodeSelections(actualNodes, nodeIds);
},

View File

@@ -1,5 +1,5 @@
import {
WatchSource, watch, shallowRef,
watch, shallowRef, type Ref,
} from 'vue';
import { injectKey } from '@/presentation/injectionSymbols';
import { IEventSubscription } from '@/infrastructure/Events/IEventSource';
@@ -11,31 +11,31 @@ import { TreeNodeStateDescriptor } from './Node/State/StateDescriptor';
export type NodeStateChangeEventCallback = (args: NodeStateChangeEventArgs) => void;
export function useNodeStateChangeAggregator(
treeWatcher: WatchSource<TreeRoot>,
treeRootRef: Readonly<Ref<TreeRoot>>,
useTreeNodes = useCurrentTreeNodes,
) {
const { nodes } = useTreeNodes(treeWatcher);
const { nodes } = useTreeNodes(treeRootRef);
const { events } = injectKey((keys) => keys.useAutoUnsubscribedEvents);
const onNodeChangeCallback = shallowRef<NodeStateChangeEventCallback>();
watch([
() => nodes.value,
() => onNodeChangeCallback.value,
], ([newNodes, callback]) => {
if (!callback) { // might not be registered yet
return;
}
if (!newNodes || newNodes.flattenedNodes.length === 0) {
events.unsubscribeAll();
return;
}
const allNodes = newNodes.flattenedNodes;
events.unsubscribeAllAndRegister(
subscribeToNotifyOnFutureNodeChanges(allNodes, callback),
);
notifyCurrentNodeState(allNodes, callback);
});
watch(
[nodes, onNodeChangeCallback],
([newNodes, callback]) => {
if (!callback) { // may not be registered yet
return;
}
if (!newNodes || newNodes.flattenedNodes.length === 0) {
events.unsubscribeAll();
return;
}
const allNodes = newNodes.flattenedNodes;
events.unsubscribeAllAndRegister(
subscribeToNotifyOnFutureNodeChanges(allNodes, callback),
);
notifyCurrentNodeState(allNodes, callback);
},
);
function onNodeStateChange(
callback: NodeStateChangeEventCallback,

View File

@@ -8,14 +8,16 @@ import { TreeNode } from './Node/TreeNode';
type TreeNavigationKeyCodes = 'ArrowLeft' | 'ArrowUp' | 'ArrowRight' | 'ArrowDown' | ' ' | 'Enter';
export function useTreeKeyboardNavigation(
treeRoot: TreeRoot,
treeElementRef: Ref<HTMLElement | undefined>,
treeRootRef: Readonly<Ref<TreeRoot>>,
treeElementRef: Readonly<Ref<HTMLElement | undefined>>,
) {
useKeyboardListener(treeElementRef, (event) => {
if (!treeElementRef.value) {
return; // Not yet initialized?
}
const treeRoot = treeRootRef.value;
const keyCode = event.key as TreeNavigationKeyCodes;
if (!treeRoot.focus.currentSingleFocusedNode) {
@@ -39,7 +41,7 @@ export function useTreeKeyboardNavigation(
}
function useKeyboardListener(
elementRef: Ref<HTMLElement | undefined>,
elementRef: Readonly<Ref<HTMLElement | undefined>>,
handleKeyboardEvent: (event: KeyboardEvent) => void,
) {
onMounted(() => {

View File

@@ -1,4 +1,4 @@
import { WatchSource, watch } from 'vue';
import { type Ref, watch } from 'vue';
import { TreeViewFilterAction, TreeViewFilterEvent, TreeViewFilterPredicate } from './Bindings/TreeInputFilterEvent';
import { ReadOnlyTreeNode, TreeNode } from './Node/TreeNode';
import { TreeRoot } from './TreeRoot/TreeRoot';
@@ -8,18 +8,18 @@ import { TreeNodeStateTransaction } from './Node/State/StateAccess';
import { TreeNodeStateDescriptor } from './Node/State/StateDescriptor';
export function useTreeQueryFilter(
latestFilterEventWatcher: WatchSource<TreeViewFilterEvent | undefined>,
treeWatcher: WatchSource<TreeRoot>,
latestFilterEventRef: Readonly<Ref<TreeViewFilterEvent | undefined>>,
treeRootRef: Readonly<Ref<TreeRoot>>,
) {
const { nodes } = useCurrentTreeNodes(treeWatcher);
const { nodes } = useCurrentTreeNodes(treeRootRef);
let isFiltering = false;
const statesBeforeFiltering = new NodeStateRestorer();
statesBeforeFiltering.saveStateBeforeFilter(nodes.value);
setupWatchers({
filterEventWatcher: latestFilterEventWatcher,
nodesWatcher: () => nodes.value,
filterEventRef: latestFilterEventRef,
nodesRef: nodes,
onFilterTrigger: (predicate, newNodes) => runFilter(
newNodes,
predicate,
@@ -171,18 +171,18 @@ class NodeStateRestorer {
}
function setupWatchers(options: {
filterEventWatcher: WatchSource<TreeViewFilterEvent | undefined>,
nodesWatcher: WatchSource<QueryableNodes>,
onFilterReset: () => void,
onFilterTrigger: (
readonly filterEventRef: Readonly<Ref<TreeViewFilterEvent | undefined>>,
readonly nodesRef: Readonly<Ref<QueryableNodes>>,
readonly onFilterReset: () => void,
readonly onFilterTrigger: (
predicate: TreeViewFilterPredicate,
nodes: QueryableNodes,
) => void,
}) {
watch(
[
options.filterEventWatcher,
options.nodesWatcher,
options.filterEventRef,
options.nodesRef,
],
([filterEvent, nodes]) => {
if (!filterEvent) {

View File

@@ -1,6 +1,5 @@
import {
WatchSource, computed,
ref, watch,
type Ref, computed, shallowReadonly,
} from 'vue';
import { ICategoryCollection } from '@/domain/ICategoryCollection';
import { injectKey } from '@/presentation/injectionSymbols';
@@ -10,7 +9,7 @@ import { convertToNodeInput } from './TreeNodeMetadataConverter';
import { parseSingleCategory, parseAllCategories } from './CategoryNodeMetadataConverter';
export function useTreeViewNodeInput(
categoryIdWatcher: WatchSource<number | undefined>,
categoryIdRef: Readonly<Ref<number | undefined>>,
parser: CategoryNodeParser = {
parseSingle: parseSingleCategory,
parseAll: parseAllCategories,
@@ -19,20 +18,14 @@ export function useTreeViewNodeInput(
) {
const { currentState } = injectKey((keys) => keys.useCollectionState);
const categoryId = ref<number | undefined>();
watch(categoryIdWatcher, (newCategoryId) => {
categoryId.value = newCategoryId;
}, { immediate: true });
const nodes = computed<readonly TreeInputNodeData[]>(() => {
const nodeMetadataList = parseNodes(categoryId.value, currentState.value.collection, parser);
const nodeMetadataList = parseNodes(categoryIdRef.value, currentState.value.collection, parser);
const nodeInputs = nodeMetadataList.map((node) => nodeConverter(node));
return nodeInputs;
});
return {
treeViewInputNodes: nodes,
treeViewInputNodes: shallowReadonly(nodes),
};
}