Skip to content

Commit

Permalink
fix: fix node component failing to update on property changes (#14)
Browse files Browse the repository at this point in the history
jaredLunde authored Apr 25, 2022
1 parent 4bcc936 commit 1bde4fd
Showing 18 changed files with 303 additions and 180 deletions.
7 changes: 7 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -449,6 +449,13 @@ interface UseSelectionsPlugin {
* Clear all of the selections
*/
clear(): void;
/**
* A utility function that yields nodes from a set of selections if they
* don't have a parent node in the set.
*
* @yields {number} - A node id
*/
narrow(): Generator<number, void, unknown>;
}
```

13 changes: 9 additions & 4 deletions examples/basic/src/App.tsx
Original file line number Diff line number Diff line change
@@ -50,9 +50,10 @@ export default function App() {

const nodeIds: number[] = [event.dir.id];
const nodes = event.dir.nodes ? [...event.dir.nodes] : [];
let nodeId: number | undefined;

while (nodes.length) {
const node = tree.getById(nodes.pop() ?? -1);
while ((nodeId = nodes.pop())) {
const node = tree.getById(nodeId);

if (node) {
nodeIds.push(node.id);
@@ -66,7 +67,7 @@ export default function App() {
traits.set("drop-target", nodeIds);
} else if (event.type === "drop") {
traits.clear("drop-target");
const selected = selections.didChange.getState();
const selected = new Set(selections.narrow());

if (
event.node === event.dir ||
@@ -81,13 +82,17 @@ export default function App() {
await tree.expand(event.dir);
}

const promises: Promise<void>[] = [];

for (const id of selected) {
const node = tree.getById(id);

if (node) {
await tree.move(node, event.dir);
promises.push(tree.move(node, event.dir));
}
}

await Promise.all(promises);
};

moveSelections();
31 changes: 17 additions & 14 deletions src/node.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as React from "react";
import { isDir } from "./file-tree";
import { isDir, isFile } from "./file-tree";
import type { FileTree, FileTreeNode } from "./file-tree";
import { subject } from "./tree/subject";
import type { NodePlugin } from "./use-node-plugins";
@@ -33,17 +33,20 @@ export function Node<Meta>(props: NodeProps<Meta>) {
* @param config - Props to generate exploration node-specific props from
*/
export function useNodeProps<Meta>(config: Omit<NodeProps<Meta>, "as">) {
return React.useMemo<React.HTMLAttributes<HTMLElement>>(() => {
const node = config.node;
const dir = isDir(node);
const { node, tree, index, style } = config;
const type = isDir(node) ? "dir" : isFile(node) ? "file" : "prompt";
const expanded = isDir(node) ? node.expanded : undefined;
const { id, depth } = node;

return React.useMemo<React.HTMLAttributes<HTMLElement>>(() => {
return {
role: "button",
style: config.style,
"data-exploration-id": node.id,
"data-exploration-index": config.index,
"data-exploration-depth": node.depth,
"data-exploration-expanded": dir ? node.expanded : undefined,
style,
"data-exploration-id": id,
"data-exploration-index": index,
"data-exploration-depth": depth,
"data-exploration-type": type,
"data-exploration-expanded": expanded,
onClick(event) {
event.currentTarget.focus();

@@ -57,16 +60,16 @@ export function useNodeProps<Meta>(config: Omit<NodeProps<Meta>, "as">) {
return;
}

if (dir) {
if (node.expanded) {
config.tree.collapse(node);
if (isDir(node)) {
if (expanded) {
tree.collapse(node);
} else {
config.tree.expand(node);
tree.expand(node);
}
}
},
};
}, [config.index, config.style, config.tree, config.node]);
}, [index, depth, expanded, style, type, node, tree, id]);
}

const empty: [] = [];
3 changes: 1 addition & 2 deletions src/use-deferred-value.ts
Original file line number Diff line number Diff line change
@@ -3,7 +3,6 @@ import {
requestTimeout,
} from "@essentials/request-timeout";
import * as React from "react";
import { perf } from "./utils";

export const useDeferredValue: <T>(
value: T,
@@ -25,7 +24,7 @@ export function useThrottle<T>(

React.useEffect(() => {
let didUnmount = false;
const now = perf.now();
const now = performance.now();
waitedFor.current +=
now - (lastInvocation.current === 0 ? now : lastInvocation.current);
lastInvocation.current = now;
2 changes: 1 addition & 1 deletion src/use-dnd.test.tsx
Original file line number Diff line number Diff line change
@@ -17,7 +17,7 @@ describe("useDnd()", () => {
beforeAll(() => {
// this is here to silence a warning temporarily
// we'll fix it in the next exercise
jest.spyOn(console, "error").mockImplementation((...args) => {});
jest.spyOn(console, "error").mockImplementation(() => {});
});

afterAll(() => {
27 changes: 15 additions & 12 deletions src/use-dnd.ts
Original file line number Diff line number Diff line change
@@ -18,7 +18,10 @@ import { shallowEqual } from "./utils";
* @param config.dragOverExpandTimeout - Timeout for expanding a directory when a draggable
* element enters it.
*/
export function useDnd(fileTree: FileTree, config: UseDndConfig): UseDndPlugin {
export function useDnd<Meta>(
fileTree: FileTree<Meta>,
config: UseDndConfig
): UseDndPlugin<Meta> {
const storedConfig = React.useRef(config);
const dnd = React.useMemo(() => createDnd(fileTree), [fileTree]);
const storedTimeout = React.useRef<{
@@ -149,10 +152,10 @@ const createProps = trieMemoize(

onDragEnter() {
const dir = isDir(node) ? node : node.parent;
const snapshot = dnd.getState();
const state = dnd.getState();

if (dir && snapshot?.node) {
dnd.setState({ type: "enter", node: snapshot.node, dir });
if (dir && state?.node) {
dnd.setState({ type: "enter", node: state.node, dir });
}
},

@@ -162,19 +165,19 @@ const createProps = trieMemoize(

onDragLeave() {
const dir = isDir(node) ? node : node.parent;
const snapshot = dnd.getState();
const state = dnd.getState();

if (dir && snapshot && snapshot.type === "enter" && snapshot.node) {
dnd.setState({ type: "leave", node: snapshot.node, dir });
if (dir && state && state.type === "enter" && state.node) {
dnd.setState({ type: "leave", node: state.node, dir });
}
},

onDrop() {
const dir = isDir(node) ? node : node.parent;
const snapshot = dnd.getState();
const state = dnd.getState();

if (dir && snapshot?.node) {
dnd.setState({ type: "drop", node: snapshot.node, dir });
if (dir && state?.node) {
dnd.setState({ type: "drop", node: state.node, dir });
}
},
})
@@ -266,11 +269,11 @@ export interface UseDndConfig {
windowRef: WindowRef;
}

export interface UseDndPlugin {
export interface UseDndPlugin<Meta> {
/**
* A subject that emits drag 'n drop events.
*/
didChange: Subject<DndEvent<any> | null>;
didChange: Subject<DndEvent<Meta> | null>;
/**
* Get the drag 'n drop props for a given node ID.
*/
2 changes: 1 addition & 1 deletion src/use-hotkeys.test.tsx
Original file line number Diff line number Diff line change
@@ -21,7 +21,7 @@ describe("useHotkeys()", () => {
beforeAll(() => {
// this is here to silence a warning temporarily
// we'll fix it in the next exercise
jest.spyOn(console, "error").mockImplementation((...args) => {});
jest.spyOn(console, "error").mockImplementation(() => {});
});

afterAll(() => {
4 changes: 1 addition & 3 deletions src/use-node-plugins.ts
Original file line number Diff line number Diff line change
@@ -2,15 +2,14 @@ import * as React from "react";
import trieMemoize from "trie-memoize";
import { useSyncExternalStore } from "use-sync-external-store/shim";
import type { Subject } from "./tree/subject";
import { mergeProps as mergeProps_, throttle } from "./utils";
import { mergeProps as mergeProps_ } from "./utils";

/**
* A hook that observes to plugins and retrieves props that should be applied
* to a given node. An example of a plugin wouuld be the `useTraits()` hook.
*
* @param nodeId - The node ID used to retrieve props from a plugin
* @param plugins - A list of file tree plugins
* @returns A memoized set of props returned by the plugins
* @example
* ```ts
* const traits = useTraits(fileTree)
@@ -49,7 +48,6 @@ export function useNodePlugins(

return useSyncExternalStore(
(callback) => {
callback = throttle(callback, 60);
const unsubs: (() => void)[] = new Array(plugins.length);

for (let i = 0; i < numPlugins; i++) {
8 changes: 4 additions & 4 deletions src/use-resize-observer.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { fireEvent } from "@testing-library/react";
import { act, renderHook } from "@testing-library/react-hooks";
import { renderHook } from "@testing-library/react-hooks";
import { useResizeObserver } from "./use-resize-observer";

class ResizeObserverPolyfill {
@@ -57,7 +57,7 @@ describe("useResizeObserver()", () => {
ResizeObserver: ResizeObserverPolyfill,
})
);
const b = renderHook(() =>
renderHook(() =>
useResizeObserver(ref, callbackB, {
ResizeObserver: ResizeObserverPolyfill,
})
@@ -82,12 +82,12 @@ describe("useResizeObserver()", () => {
const refA = { current: elementA };
const refB = { current: elementB };

const a = renderHook(() =>
renderHook(() =>
useResizeObserver(refA, callbackA, {
ResizeObserver: ResizeObserverPolyfill,
})
);
const b = renderHook(() =>
renderHook(() =>
useResizeObserver(refB, callbackB, {
ResizeObserver: ResizeObserverPolyfill,
})
4 changes: 2 additions & 2 deletions src/use-roving-focus.ts
Original file line number Diff line number Diff line change
@@ -40,11 +40,11 @@ const createProps = trieMemoize(
return {
tabIndex: focused ? 0 : -1,

onFocus(e: React.FocusEvent<HTMLElement>) {
onFocus() {
focusedNodeId.setState(nodeId);
},

onBlur(e: React.FocusEvent<HTMLElement>) {
onBlur() {
if (focused) {
focusedNodeId.setState(-1);
}
37 changes: 37 additions & 0 deletions src/use-selections.ts
Original file line number Diff line number Diff line change
@@ -2,6 +2,7 @@ import * as React from "react";
import trieMemoize from "trie-memoize";
import type { FileTree } from "./file-tree";
import { SubjectSet } from "./observable-data";
import { nodesById } from "./tree/nodes-by-id";
import type { Subject } from "./tree/subject";
import { useVisibleNodes } from "./use-visible-nodes";

@@ -66,6 +67,35 @@ export function useSelections<Meta>(
clear() {
selectionsSet.clear();
},

narrow: function* () {
// Remove child nodes from selections if their parent is already selected
for (const nodeId of selectionsSet) {
const node = nodesById[nodeId];

if (node) {
let parentId = node.parentId;

while (parentId > -1) {
if (selectionsSet.has(parentId)) {
break;
}

const parentNode = nodesById[parentId];

if (!parentNode) {
break;
}

parentId = parentNode.parentId;
}

if (parentId === -1) {
yield nodeId;
}
}
}
},
};
}

@@ -222,4 +252,11 @@ export interface UseSelectionsPlugin {
* Clear all of the selections
*/
clear(): void;
/**
* A utility function that yields nodes from a set of selections if they
* don't have a parent node in the set.
*
* @yields {number} - A node id
*/
narrow(): Generator<number, void, unknown>;
}
40 changes: 39 additions & 1 deletion src/utils.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { mergeProps } from ".";
import { shallowEqual } from "./utils";
import { shallowEqual, throttle } from "./utils";

describe("shallowEqual()", () => {
it("should return true for equal objects", () => {
@@ -78,3 +78,41 @@ describe("mergeProps()", () => {
expect(merged.style).toEqual({ backgroundColor: "blue" });
});
});

describe("throttle()", () => {
it("should invoke the callback once per timeout", () => {
const fn = jest.fn((value: number) => {});
const throttled = throttle(fn, 30);

throttled(1);
expect(fn).toHaveBeenCalledTimes(0);

jest.advanceTimersByTime(1000 / 60);

throttled(2);
expect(fn).toHaveBeenCalledTimes(0);

jest.advanceTimersByTime(1000 / 60);
expect(fn).toHaveBeenCalledTimes(1);
expect(fn).lastCalledWith(2);
});

it("should fire a leading call", () => {
const fn = jest.fn((value: number) => {});
const throttled = throttle(fn, 30, true);

throttled(1);
expect(fn).toHaveBeenCalledTimes(1);
expect(fn).lastCalledWith(1);

throttled(2);
expect(fn).toHaveBeenCalledTimes(1);
expect(fn).lastCalledWith(1);
jest.advanceTimersByTime(1000 / 60);

throttled(3);
jest.advanceTimersByTime(1000 / 60);
expect(fn).toHaveBeenCalledTimes(2);
expect(fn).lastCalledWith(3);
});
});
21 changes: 13 additions & 8 deletions src/utils.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
// @react-aria/utils
// Credit: https://github.com/adobe/react-spectrum/tree/main/packages/%40react-aria
import {
clearRequestTimeout,
requestTimeout,
} from "@essentials/request-timeout";
import clsx from "clsx";

/**
@@ -84,38 +88,39 @@ export function throttle<CallbackArguments extends any[]>(
): (...args: CallbackArguments) => void {
const ms = 1000 / fps;
let prev = 0;
let trailingTimeout: ReturnType<typeof setTimeout>;
const clearTrailing = () => trailingTimeout && clearTimeout(trailingTimeout);
let trailingTimeout: ReturnType<typeof requestTimeout>;
const clearTrailing = () =>
trailingTimeout && clearRequestTimeout(trailingTimeout);

return function () {
// eslint-disable-next-line prefer-rest-params
const args = arguments;
const rightNow = perf.now();
const rightNow = performance.now();
const call = () => {
prev = rightNow;
clearTrailing();
// @ts-expect-error: IArguments isn't assignable, but they're the same thing
// eslint-disable-next-line prefer-spread
callback.apply(null, args as any);
callback.apply(null, args);
};
const current = prev;
// leading
if (leading && current === 0) return call();
const delta = rightNow - current;
// body
if (rightNow - current > ms) {
if (current > 0) return call();
prev = rightNow;
}
// trailing
clearTrailing();
trailingTimeout = setTimeout(() => {
trailingTimeout = requestTimeout(() => {
call();
prev = 0;
}, ms);
}, ms - delta);
};
}

export const perf = typeof performance !== "undefined" ? performance : Date;

export function shallowEqual<
A extends Record<string | number | symbol, unknown> | null,
B extends Record<string | number | symbol, unknown> | null
7 changes: 7 additions & 0 deletions test/setup.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// This file is for setting up Jest test environments
import "@testing-library/jest-dom/extend-expect";
import { clearRequestTimeout } from "@essentials/request-timeout";

afterEach(() => {
jest.clearAllMocks();
@@ -14,3 +15,9 @@ window.requestAnimationFrame = (callback) => {
window.cancelAnimationFrame = (id) => {
clearTimeout(id);
};

jest.mock("@essentials/request-timeout", () => ({
default: window.setTimeout,
requestTimeout: window.setTimeout,
clearRequestTimeout: window.clearTimeout,
}));
2 changes: 1 addition & 1 deletion types/tsconfig.tsbuildinfo

Large diffs are not rendered by default.

167 changes: 88 additions & 79 deletions types/use-dnd.d.ts
Original file line number Diff line number Diff line change
@@ -12,87 +12,96 @@ import type { WindowRef } from "./types";
* @param config.dragOverExpandTimeout - Timeout for expanding a directory when a draggable
* element enters it.
*/
export declare function useDnd(fileTree: FileTree, config: UseDndConfig): UseDndPlugin;
export declare type DndEvent<Meta> = {
type: "start";
/**
* The node that is being dragged
*/
node: FileTreeNode<Meta>;
} | {
type: "end";
/**
* The node that is being dragged
*/
node: FileTreeNode<Meta>;
} | {
type: "enter";
/**
* The node that is being dragged
*/
node: FileTreeNode<Meta>;
/**
* The directory that the node is being dragged over
*/
dir: Dir<Meta>;
} | {
type: "expanded";
/**
* The node that is being dragged
*/
node: FileTreeNode<Meta>;
/**
* The directory that the node is being dragged over
*/
dir: Dir<Meta>;
} | {
type: "leave";
/**
* The node that is being dragged
*/
node: FileTreeNode<Meta>;
/**
* The directory that the node was being dragged over
*/
dir: Dir<Meta>;
} | {
type: "drop";
/**
* The node that is being dragged
*/
node: FileTreeNode<Meta>;
/**
* The directory that the node is being dragged over
*/
dir: Dir<Meta>;
};
export declare function useDnd<Meta>(
fileTree: FileTree<Meta>,
config: UseDndConfig
): UseDndPlugin<Meta>;
export declare type DndEvent<Meta> =
| {
type: "start";
/**
* The node that is being dragged
*/
node: FileTreeNode<Meta>;
}
| {
type: "end";
/**
* The node that is being dragged
*/
node: FileTreeNode<Meta>;
}
| {
type: "enter";
/**
* The node that is being dragged
*/
node: FileTreeNode<Meta>;
/**
* The directory that the node is being dragged over
*/
dir: Dir<Meta>;
}
| {
type: "expanded";
/**
* The node that is being dragged
*/
node: FileTreeNode<Meta>;
/**
* The directory that the node is being dragged over
*/
dir: Dir<Meta>;
}
| {
type: "leave";
/**
* The node that is being dragged
*/
node: FileTreeNode<Meta>;
/**
* The directory that the node was being dragged over
*/
dir: Dir<Meta>;
}
| {
type: "drop";
/**
* The node that is being dragged
*/
node: FileTreeNode<Meta>;
/**
* The directory that the node is being dragged over
*/
dir: Dir<Meta>;
};
export interface DndProps {
draggable: true;
onDragStart: React.MouseEventHandler<HTMLElement>;
onDragEnd: React.MouseEventHandler<HTMLElement>;
onDragOver: React.MouseEventHandler<HTMLElement>;
onDragEnter: React.MouseEventHandler<HTMLElement>;
onDragLeave: React.MouseEventHandler<HTMLElement>;
onDrop: React.MouseEventHandler<HTMLElement>;
draggable: true;
onDragStart: React.MouseEventHandler<HTMLElement>;
onDragEnd: React.MouseEventHandler<HTMLElement>;
onDragOver: React.MouseEventHandler<HTMLElement>;
onDragEnter: React.MouseEventHandler<HTMLElement>;
onDragLeave: React.MouseEventHandler<HTMLElement>;
onDrop: React.MouseEventHandler<HTMLElement>;
}
export interface UseDndConfig {
/**
* Timeout for expanding a directory when a draggable element enters it.
*/
dragOverExpandTimeout?: number;
/**
* A React ref created by useRef() or an HTML element for the container viewport
* you're rendering the list inside of.
*/
windowRef: WindowRef;
/**
* Timeout for expanding a directory when a draggable element enters it.
*/
dragOverExpandTimeout?: number;
/**
* A React ref created by useRef() or an HTML element for the container viewport
* you're rendering the list inside of.
*/
windowRef: WindowRef;
}
export interface UseDndPlugin {
/**
* A subject that emits drag 'n drop events.
*/
didChange: Subject<DndEvent<any> | null>;
/**
* Get the drag 'n drop props for a given node ID.
*/
getProps: (nodeId: number) => DndProps | React.HTMLAttributes<HTMLElement>;
export interface UseDndPlugin<Meta> {
/**
* A subject that emits drag 'n drop events.
*/
didChange: Subject<DndEvent<Meta> | null>;
/**
* Get the drag 'n drop props for a given node ID.
*/
getProps: (nodeId: number) => DndProps | React.HTMLAttributes<HTMLElement>;
}
26 changes: 14 additions & 12 deletions types/use-node-plugins.d.ts
Original file line number Diff line number Diff line change
@@ -6,24 +6,26 @@ import type { Subject } from "./tree/subject";
*
* @param nodeId - The node ID used to retrieve props from a plugin
* @param plugins - A list of file tree plugins
* @returns A memoized set of props returned by the plugins
* @example
* ```ts
* const traits = useTraits(fileTree)
* const props = useNodePlugins(fileTree.visibleNodes[0], [traits])
* return <div {...props}>...</div>
* ```
*/
export declare function useNodePlugins(nodeId: number, plugins?: NodePlugin[]): React.HTMLAttributes<HTMLElement>;
export declare function useNodePlugins(
nodeId: number,
plugins?: NodePlugin[]
): React.HTMLAttributes<HTMLElement>;
export declare type NodePlugin<T = unknown> = {
/**
* A subject that the `useNodePlugins()` hook will observe to.
*/
didChange: Subject<T>;
/**
* A function that returns React props based on a node ID.
*
* @param nodeId - The ID of a node in the file tree.
*/
getProps(nodeId: number): React.HTMLAttributes<HTMLElement>;
/**
* A subject that the `useNodePlugins()` hook will observe to.
*/
didChange: Subject<T>;
/**
* A function that returns React props based on a node ID.
*
* @param nodeId - The ID of a node in the file tree.
*/
getProps(nodeId: number): React.HTMLAttributes<HTMLElement>;
};
82 changes: 46 additions & 36 deletions types/use-selections.d.ts
Original file line number Diff line number Diff line change
@@ -9,43 +9,53 @@ import type { Subject } from "./tree/subject";
* nodes to this option. By default, `useVirtualize()` uses the nodes returned by
* `useVisibleNodes()`
*/
export declare function useSelections<Meta>(fileTree: FileTree<Meta>, nodes?: number[]): UseSelectionsPlugin;
export declare function useSelections<Meta>(
fileTree: FileTree<Meta>,
nodes?: number[]
): UseSelectionsPlugin;
export interface SelectionsProps {
onClick: React.MouseEventHandler<HTMLElement>;
onClick: React.MouseEventHandler<HTMLElement>;
}
export interface UseSelectionsPlugin {
/**
* A subject that you can use to observe to changes to selections.
*/
didChange: Subject<Set<number>>;
/**
* Get the React props for a given node ID.
*
* @param nodeId - A node ID
*/
getProps(nodeId: number): SelectionsProps;
/**
* The head of the selections list
*/
get head(): number | null;
/**
* The tail of the selections list
*/
get tail(): number | null;
/**
* Select given node ids
*
* @param nodeIds - Node IDs
*/
select(...nodeIds: number[]): void;
/**
* Deselect given node ids
*
* @param nodeIds - Node IDs
*/
deselect(...nodeIds: number[]): void;
/**
* Clear all of the selections
*/
clear(): void;
/**
* A subject that you can use to observe to changes to selections.
*/
didChange: Subject<Set<number>>;
/**
* Get the React props for a given node ID.
*
* @param nodeId - A node ID
*/
getProps(nodeId: number): SelectionsProps;
/**
* The head of the selections list
*/
get head(): number | null;
/**
* The tail of the selections list
*/
get tail(): number | null;
/**
* Select given node ids
*
* @param nodeIds - Node IDs
*/
select(...nodeIds: number[]): void;
/**
* Deselect given node ids
*
* @param nodeIds - Node IDs
*/
deselect(...nodeIds: number[]): void;
/**
* Clear all of the selections
*/
clear(): void;
/**
* A utility function that yields nodes from a set of selections if they
* don't have a parent node in the set.
*
* @yields {number} - A node id
*/
narrow(): Generator<number, void, unknown>;
}

0 comments on commit 1bde4fd

Please sign in to comment.