Skip to content

Commit

Permalink
refactor: Define PrimerNode
Browse files Browse the repository at this point in the history 
As described in source comments, this is similar to ReactFlow's `Node`, but more type-safe. We are able to statically assert that the `data` fields of our nodes match what we tell ReactFlow to expect.

`Equals` and `assertType` are inspired by, respectively, microsoft/TypeScript#27024 (comment) and microsoft/TypeScript#27024.

We carry on using standard ReactFlow edges and handles, for now. Though we will likely do something similar there in future, once we require further customization. In fact, we now define a very simple `PrimerEdge` type for a small degree of forwards-compatibility.

This also puts us in slightly better shape to ditch ReactFlow for an alternative, should we wish to, since we now implement better abstractions in some ways ourselves, using ReactFlow's types less extensively.
  • Loading branch information
@georgefst
georgefst committed Mar 13, 2023
1 parent 5fc85e4 commit e49c6e8
Show file tree
Hide file tree
Showing 4 changed files with 102 additions and 50 deletions.
38 changes: 20 additions & 18 deletions src/components/TreeReactFlow/Types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
NodeFlavorTextBody,
NodeType,
} from "@/primer-api";
import { Edge, Node } from "reactflow";
import { Edge } from "reactflow";
import { unzip } from "fp-ts/lib/Array";

/** A generic graph. */
Expand All @@ -28,15 +28,9 @@ export const combineGraphs = <
return { nodes: nodes.flat(), edges: edges.flat() };
};

export type PrimerGraph<T> = Graph<
Node<PrimerNodeProps<T> | PrimerDefNameNodeProps>,
Edge<Empty>
>;
export type PrimerGraph<T> = Graph<Positioned<PrimerNode<T>>, PrimerEdge>;

export type PrimerGraphNoPos<T> = Graph<
NodeNoPos<PrimerNodeProps<T> | PrimerDefNameNodeProps>,
Edge<Empty>
>;
export type PrimerGraphNoPos<T> = Graph<PrimerNode<T>, PrimerEdge>;

/** A generic edge-labelled tree. */
export type TreeSimple<N, E> = {
Expand Down Expand Up @@ -96,15 +90,21 @@ export const treeToGraph = <
);
};

export type PrimerTree<T> = TreeSimple<
Node<PrimerNodeProps<T> | PrimerDefNameNodeProps>,
Edge<Empty>
>;
export type PrimerTree<T> = TreeSimple<Positioned<PrimerNode<T>>, PrimerEdge>;

export type PrimerTreeNoPos<T> = TreeSimple<
NodeNoPos<PrimerNodeProps<T> | PrimerDefNameNodeProps>,
Edge<Empty>
>;
export type PrimerTreeNoPos<T> = TreeSimple<PrimerNode<T>, PrimerEdge>;

export type PrimerEdge = Edge<Empty>;

/** Our node type. `Positioned<PrimerNode<T>>` can be safely cast to a ReactFlow `Node`.
* This is more type safe than using ReactFlow's types directly: this way we can ensure that
* the `type` field always corresponds to a custom node type we've registered with ReactFlow,
* and that `data` contains the expected type of data for that type of custom node.
*/
export type PrimerNode<T> = { id: string } & (
| { type: "primer"; data: PrimerNodeProps<T> }
| { type: "primer-def-name"; data: PrimerDefNameNodeProps }
);

/** Node properties. */
export type PrimerNodeProps<T> = {
Expand Down Expand Up @@ -140,7 +140,9 @@ export type PrimerDefNameNodeProps = {
height: number;
};

export type NodeNoPos<T> = Omit<Node<T>, "position">;
export type Positioned<T> = T & {
position: { x: number; y: number };
};

/** The empty record (note that `{}` is something different: https://typescript-eslint.io/rules/ban-types/) */
export type Empty = Record<string, never>;
75 changes: 55 additions & 20 deletions src/components/TreeReactFlow/index.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import { Def, Tree, Selection } from "@/primer-api";
import {
ReactFlow,
Edge,
Node,
Node as RFNode,
Handle,
Position,
NodeProps,
Expand All @@ -20,10 +19,11 @@ import {
PrimerTreeProps,
PrimerTreeNoPos,
PrimerTreePropsOne,
Empty,
treeToGraph,
NodeNoPos,
PrimerDefNameNodeProps,
PrimerNode,
PrimerEdge,
Positioned,
} from "./Types";
import { layoutTree } from "./layoutTree";
import deepEqual from "deep-equal";
Expand All @@ -36,6 +36,7 @@ import {
flavorLabelClasses,
noBodyFlavorContents,
} from "./Flavor";
import { assertType, Equal } from "@/util";

type NodeParams = {
nodeWidth: number;
Expand All @@ -47,21 +48,18 @@ export type TreeReactFlowProps = {
defs: Def[];
onNodeClick?: (
event: React.MouseEvent,
node: Node<PrimerNodeProps<PrimerTreeProps> | PrimerDefNameNodeProps>
node: Positioned<PrimerNode<PrimerTreeProps>>
) => void;
treePadding: number;
forestLayout: "Horizontal" | "Vertical";
} & NodeParams;

const primerNodeTypeName = "primer";
const primerDefNameNodeTypeName = "primer-def-name";

const handle = (type: HandleType, position: Position) => (
<Handle id={position} isConnectable={false} type={type} position={position} />
);

const nodeTypes = {
[primerNodeTypeName]: <T,>(p: NodeProps<PrimerNodeProps<T>>) => (
primer: <T,>(p: NodeProps<PrimerNodeProps<T>>) => (
<>
{handle("target", Position.Top)}
{handle("target", Position.Left)}
Expand Down Expand Up @@ -103,7 +101,7 @@ const nodeTypes = {
{handle("source", Position.Right)}
</>
),
[primerDefNameNodeTypeName]: (p: NodeProps<PrimerDefNameNodeProps>) => (
"primer-def-name": (p: NodeProps<PrimerDefNameNodeProps>) => (
<>
<div
className={classNames(
Expand All @@ -127,6 +125,20 @@ const nodeTypes = {
),
};

// Check that `nodeTypes` is in sync with `PrimerNode`,
// i.e. that we register our custom nodes correctly (see `PrimerNode` for further explanation).
assertType<
Equal<
PrimerNode<unknown>,
{ id: string } & {
[T in keyof typeof nodeTypes]: {
type: T;
data: Parameters<(typeof nodeTypes)[T]>[0]["data"];
};
}[keyof typeof nodeTypes]
>
>;

const augmentTree = async <T,>(
tree: Tree,
p: NodeParams & T
Expand All @@ -144,7 +156,7 @@ const augmentTree = async <T,>(
const [data, nested] = await nodeProps(tree, p);
const makeEdge = (
child: PrimerTreeNoPos<T>
): [PrimerTreeNoPos<T>, Edge<Empty>] => [
): [PrimerTreeNoPos<T>, PrimerEdge] => [
child,
{
id: JSON.stringify([tree.nodeId, child.node.id]),
Expand All @@ -162,7 +174,7 @@ const augmentTree = async <T,>(
childTrees: childTrees.map((e) => makeEdge(e)),
node: {
id: tree.nodeId,
type: primerNodeTypeName,
type: "primer",
data,
},
},
Expand Down Expand Up @@ -277,7 +289,7 @@ export const TreeReactFlow = (p: TreeReactFlowProps) => {
const defNodeId = "def-" + def.name.baseName;
const sigEdgeId = "def-sig-" + def.name.baseName;
const bodyEdgeId = "def-body-" + def.name.baseName;
const defNameNode: NodeNoPos<PrimerDefNameNodeProps> = {
const defNameNode: PrimerNode<unknown> = {
id: defNodeId,
data: {
def: def.name,
Expand All @@ -286,14 +298,14 @@ export const TreeReactFlow = (p: TreeReactFlowProps) => {
selected:
deepEqual(p.selection?.def, def.name) && !p.selection?.node,
},
type: primerDefNameNodeTypeName,
type: "primer-def-name",
};
const defEdge = async (
tree: Tree,
augmentParams: NodeParams & PrimerTreeProps,
edgeId: string
): Promise<{
subtree: [PrimerTreeNoPos<PrimerTreeProps>, Edge<Empty>];
subtree: [PrimerTreeNoPos<PrimerTreeProps>, PrimerEdge];
nested: PrimerGraph<PrimerTreeProps>[];
}> => {
const [t, nested] = await augmentTree(tree, augmentParams);
Expand Down Expand Up @@ -386,7 +398,7 @@ export const TreeReactFlow = (p: TreeReactFlowProps) => {
const id = useId();

return (
<ReactFlow
<ReactFlowSafe
id={id}
{...(p.onNodeClick && { onNodeClick: p.onNodeClick })}
nodes={nodes}
Expand All @@ -395,7 +407,7 @@ export const TreeReactFlow = (p: TreeReactFlowProps) => {
proOptions={{ hideAttribution: true, account: "paid-pro" }}
>
<Background gap={25} size={1.6} color="#81818a" />
</ReactFlow>
</ReactFlowSafe>
);
};

Expand All @@ -405,7 +417,7 @@ export type TreeReactFlowOneProps = {
tree?: Tree;
onNodeClick?: (
event: React.MouseEvent,
node: Node<PrimerNodeProps<PrimerTreePropsOne>>
node: PrimerNode<PrimerTreePropsOne>
) => void;
} & NodeParams;

Expand Down Expand Up @@ -439,7 +451,7 @@ export const TreeReactFlowOne = (p: TreeReactFlowOneProps) => {
const id = useId();

return (
<ReactFlow
<ReactFlowSafe
id={id}
{...(p.onNodeClick && { onNodeClick: p.onNodeClick })}
nodes={nodes}
Expand All @@ -448,6 +460,29 @@ export const TreeReactFlowOne = (p: TreeReactFlowOneProps) => {
proOptions={{ hideAttribution: true, account: "paid-pro" }}
>
<Background gap={25} size={1.6} color="#81818a" />
</ReactFlow>
</ReactFlowSafe>
);
};

/** A more strongly-typed version of the `ReactFlow` component.
* This allows us to use a more refined node type, and safely act on that type in handlers. */
export const ReactFlowSafe = <N extends RFNode>(
p: Omit<Parameters<typeof ReactFlow>[0], "onNodeClick" | "nodes"> & {
nodes: N[];
onNodeClick?: (e: React.MouseEvent<Element, MouseEvent>, n: N) => void;
}
): ReturnType<typeof ReactFlow> => (
<ReactFlow
{...{
...p,
onNodeClick: (e, n) => {
"onNodeClick" in p &&
p.onNodeClick(
e,
// This cast is safe because `N` is also the type of elements of the `nodes` field.
n as N
);
},
}}
></ReactFlow>
);
22 changes: 10 additions & 12 deletions src/components/TreeReactFlow/layoutTree.ts
View file
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
import { Node, Edge } from "reactflow";
import {
InnerNode as InnerTidyNode,
Node as TidyNode,
TidyLayout,
} from "@zxch3n/tidy";
import { WasmLayoutType } from "@zxch3n/tidy/wasm_dist";
import {
Empty,
NodeNoPos,
PrimerDefNameNodeProps,
PrimerNodeProps,
Positioned,
PrimerEdge,
PrimerNode,
PrimerTree,
PrimerTreeNoPos,
treeMap,
Expand Down Expand Up @@ -57,16 +55,16 @@ export const layoutTree = <T>(

type NodeInfo<T> = {
id: number;
node: NodeNoPos<PrimerNodeProps<T> | PrimerDefNameNodeProps>;
edges: { edge: Edge<Empty>; isRight: boolean }[];
node: PrimerNode<T>;
edges: { edge: PrimerEdge; isRight: boolean }[];
};
// A single node of a `PrimerTree<T>`.
// Note that this type is very similar in structure to `PrimerTree<T>`,
// the only difference being that this type does not contain the actual subtrees.
type PrimerTreeNode<T> = {
node: Node<PrimerNodeProps<T> | PrimerDefNameNodeProps>;
edges: Edge<Empty>[];
rightEdge?: Edge<Empty>;
node: Positioned<PrimerNode<T>>;
edges: PrimerEdge[];
rightEdge?: PrimerEdge;
};
const makeNodeMap = <T>(
rootId: number,
Expand Down Expand Up @@ -119,8 +117,8 @@ const primerToTidy = <T>(t: PrimerTreeNoPos<T>): [TidyNode, NodeInfo<T>[]] => {
const go = (primerTree: PrimerTreeNoPos<T>): [TidyNode, NodeInfo<T>[]] => {
const mkNodeInfos = (
primerTree0: PrimerTreeNoPos<T>[]
): [TidyNode[], NodeInfo<T>[], Edge<Empty>[]] => {
const r = primerTree0.map<[TidyNode[], NodeInfo<T>[], Edge<Empty>[]]>(
): [TidyNode[], NodeInfo<T>[], PrimerEdge[]] => {
const r = primerTree0.map<[TidyNode[], NodeInfo<T>[], PrimerEdge[]]>(
(t) => {
const [tree1, nodes1] = go(t);
// We explore (transitive) right-children now,
Expand Down
17 changes: 17 additions & 0 deletions src/util.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/** Evaluates to the type `true` when both parameters are equal, and `false` otherwise.
* NB. this actually tests mutual extendability, which is mostly a reasonable definition of
* of equality, but does mean that, for example, `any` is "equal to" everything, except `never`.
*/
export type Equal<T, S> = [T] extends [S]
? [S] extends [T]
? true
: false
: false;

/** Typechecks succesfully if and only if the input parameter is `true`.
* At runtime, this function does nothing.
*/
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
// eslint-disable-next-line @typescript-eslint/no-unused-vars, @typescript-eslint/no-empty-function
export const assertType = <T extends true>() => {};

0 comments on commit e49c6e8

Please sign in to comment.