Skip to content

Commit cbeaba6

Browse files
committed
fix: delay all state updates after mount (#158)
1 parent eefd656 commit cbeaba6

File tree

5 files changed

+47
-39
lines changed

5 files changed

+47
-39
lines changed

.changeset/heavy-carpets-smash.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"@headless-tree/react": minor
3+
"@headless-tree/core": minor
4+
---
5+
6+
all state updates (like setSelectedItems) will not propagate while the component is unmounted. This happened before for `tree.setState()` calls directly, but not individual state atoms like `setSelectedItems`. When calling `createTree()` directly (instead of `useTree()`), `tree.setMounted(true)` needs to be called once after mount. No changes are necessary when using the React-based `useTree()` integration. (#158)

packages/core/src/core/create-tree.ts

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { treeFeature } from "../features/tree/feature";
1111
import { ItemMeta } from "../features/tree/types";
1212
import { buildStaticInstance } from "./build-static-instance";
1313
import { throwError } from "../utilities/errors";
14+
import type { TreeDataRef } from "../features/main/types";
1415

1516
const verifyFeatures = (features: FeatureImplementation[] | undefined) => {
1617
const loadedFeatures = features?.map((feature) => feature.key);
@@ -161,17 +162,34 @@ export const createTree = <T>(
161162
config.setState?.(state); // TODO this cant be right... This doesnt allow external state updates
162163
// TODO this is never used, remove
163164
},
165+
setMounted: ({}, isMounted) => {
166+
const ref = treeDataRef as TreeDataRef;
167+
treeDataRef.current.isMounted = isMounted;
168+
if (isMounted) {
169+
ref.waitingForMount?.forEach((cb) => cb());
170+
ref.waitingForMount = [];
171+
}
172+
},
164173
applySubStateUpdate: <K extends keyof TreeState<any>>(
165174
{},
166175
stateName: K,
167176
updater: Updater<TreeState<T>[K]>,
168177
) => {
169-
state[stateName] =
170-
typeof updater === "function" ? updater(state[stateName]) : updater;
171-
const externalStateSetter = config[
172-
stateHandlerNames[stateName]
173-
] as Function;
174-
externalStateSetter?.(state[stateName]);
178+
const apply = () => {
179+
state[stateName] =
180+
typeof updater === "function" ? updater(state[stateName]) : updater;
181+
const externalStateSetter = config[
182+
stateHandlerNames[stateName]
183+
] as Function;
184+
externalStateSetter?.(state[stateName]);
185+
};
186+
const ref = treeDataRef.current as TreeDataRef;
187+
if (ref.isMounted) {
188+
apply();
189+
} else {
190+
ref.waitingForMount ??= [];
191+
ref.waitingForMount.push(apply);
192+
}
175193
},
176194
// TODO rebuildSubTree: (itemId: string) => void;
177195
rebuildTree: () => {

packages/core/src/features/main/types.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,11 @@ import {
1010
} from "../../types/core";
1111
import { ItemMeta } from "../tree/types";
1212

13+
export interface TreeDataRef {
14+
isMounted?: boolean;
15+
waitingForMount?: (() => void)[];
16+
}
17+
1318
export type InstanceTypeMap = {
1419
itemInstance: ItemInstance<any>;
1520
treeInstance: TreeInstance<any>;
@@ -51,6 +56,8 @@ export type MainFeatureDef<T = any> = {
5156
rebuildTree: () => void;
5257
/** @deprecated Experimental feature, might get removed or changed in the future. */
5358
scheduleRebuildTree: () => void;
59+
/** @internal */
60+
setMounted: (isMounted: boolean) => void;
5461
};
5562
itemInstance: {
5663
registerElement: (element: HTMLElement | null) => void;

packages/core/src/test-utils/test-tree.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ export class TestTree<T = string> {
7878
get instance() {
7979
if (!this.treeInstance) {
8080
this.treeInstance = createTree(this.config);
81+
this.treeInstance.setMounted(true);
8182
this.treeInstance.rebuildTree();
8283
}
8384
return this.treeInstance;

packages/react/src/use-tree.tsx

Lines changed: 9 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,42 +1,20 @@
1-
import { useEffect, useRef, useState } from "react";
1+
import { useEffect, useState } from "react";
22
import { TreeConfig, TreeState, createTree } from "@headless-tree/core";
33

4-
/* A bug with NextJS was reported in the past where very quick state updates (i.e. data
5-
* loader returning withing milliseconds) will cause the state update to occur before
6-
* mount, resulting in a console warning. This alleviates this.
7-
* We should monitor if this remains a problem in the future, maybe we can eventually
8-
* remove this workaround...
9-
*/
10-
const useApplyAfterMount = () => {
11-
const isMounted = useRef(false);
12-
const callbacks = useRef<(() => void)[]>([]);
13-
14-
useEffect(() => {
15-
isMounted.current = true;
16-
callbacks.current.forEach((callback) => callback());
17-
}, []);
18-
19-
const apply = (callback: () => void) => {
20-
if (isMounted.current) {
21-
callback();
22-
} else {
23-
callbacks.current.push(callback);
24-
}
25-
};
26-
27-
return apply;
28-
};
29-
304
export const useTree = <T,>(config: TreeConfig<T>) => {
31-
const apply = useApplyAfterMount();
325
const [tree] = useState(() => ({ current: createTree(config) }));
336
const [state, setState] = useState<Partial<TreeState<T>>>(() =>
347
tree.current.getState(),
358
);
369

3710
useEffect(() => {
11+
(tree.current as any).setMounted(true);
3812
tree.current.rebuildTree();
39-
}, [tree]); // runs only once after mount
13+
return () => {
14+
// eslint-disable-next-line react-hooks/exhaustive-deps
15+
(tree.current as any).setMounted(false);
16+
};
17+
}, [tree]);
4018

4119
tree.current.setConfig((prev) => ({
4220
...prev,
@@ -46,10 +24,8 @@ export const useTree = <T,>(config: TreeConfig<T>) => {
4624
...config.state,
4725
},
4826
setState: (state) => {
49-
apply(() => {
50-
setState(state);
51-
config.setState?.(state);
52-
});
27+
setState(state);
28+
config.setState?.(state);
5329
},
5430
}));
5531

0 commit comments

Comments
 (0)