From e49c6e8d969c600a4bc0eb5acbdaedd71a3bcc4e Mon Sep 17 00:00:00 2001 From: George Thomas Date: Thu, 2 Mar 2023 12:58:12 +0000 Subject: [PATCH] refactor: Define `PrimerNode` 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, https://github.com/microsoft/TypeScript/issues/27024#issuecomment-420227672 and https://github.com/microsoft/TypeScript/issues/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. --- src/components/TreeReactFlow/Types.ts | 38 +++++------ src/components/TreeReactFlow/index.tsx | 75 ++++++++++++++++------ src/components/TreeReactFlow/layoutTree.ts | 22 +++---- src/util.ts | 17 +++++ 4 files changed, 102 insertions(+), 50 deletions(-) create mode 100644 src/util.ts diff --git a/src/components/TreeReactFlow/Types.ts b/src/components/TreeReactFlow/Types.ts index 58015a90..0599c59e 100644 --- a/src/components/TreeReactFlow/Types.ts +++ b/src/components/TreeReactFlow/Types.ts @@ -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. */ @@ -28,15 +28,9 @@ export const combineGraphs = < return { nodes: nodes.flat(), edges: edges.flat() }; }; -export type PrimerGraph = Graph< - Node | PrimerDefNameNodeProps>, - Edge ->; +export type PrimerGraph = Graph>, PrimerEdge>; -export type PrimerGraphNoPos = Graph< - NodeNoPos | PrimerDefNameNodeProps>, - Edge ->; +export type PrimerGraphNoPos = Graph, PrimerEdge>; /** A generic edge-labelled tree. */ export type TreeSimple = { @@ -96,15 +90,21 @@ export const treeToGraph = < ); }; -export type PrimerTree = TreeSimple< - Node | PrimerDefNameNodeProps>, - Edge ->; +export type PrimerTree = TreeSimple>, PrimerEdge>; -export type PrimerTreeNoPos = TreeSimple< - NodeNoPos | PrimerDefNameNodeProps>, - Edge ->; +export type PrimerTreeNoPos = TreeSimple, PrimerEdge>; + +export type PrimerEdge = Edge; + +/** Our node type. `Positioned>` 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 = { id: string } & ( + | { type: "primer"; data: PrimerNodeProps } + | { type: "primer-def-name"; data: PrimerDefNameNodeProps } +); /** Node properties. */ export type PrimerNodeProps = { @@ -140,7 +140,9 @@ export type PrimerDefNameNodeProps = { height: number; }; -export type NodeNoPos = Omit, "position">; +export type Positioned = 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; diff --git a/src/components/TreeReactFlow/index.tsx b/src/components/TreeReactFlow/index.tsx index 21ac5d83..c4db24b6 100644 --- a/src/components/TreeReactFlow/index.tsx +++ b/src/components/TreeReactFlow/index.tsx @@ -1,8 +1,7 @@ import { Def, Tree, Selection } from "@/primer-api"; import { ReactFlow, - Edge, - Node, + Node as RFNode, Handle, Position, NodeProps, @@ -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"; @@ -36,6 +36,7 @@ import { flavorLabelClasses, noBodyFlavorContents, } from "./Flavor"; +import { assertType, Equal } from "@/util"; type NodeParams = { nodeWidth: number; @@ -47,21 +48,18 @@ export type TreeReactFlowProps = { defs: Def[]; onNodeClick?: ( event: React.MouseEvent, - node: Node | PrimerDefNameNodeProps> + node: Positioned> ) => void; treePadding: number; forestLayout: "Horizontal" | "Vertical"; } & NodeParams; -const primerNodeTypeName = "primer"; -const primerDefNameNodeTypeName = "primer-def-name"; - const handle = (type: HandleType, position: Position) => ( ); const nodeTypes = { - [primerNodeTypeName]: (p: NodeProps>) => ( + primer: (p: NodeProps>) => ( <> {handle("target", Position.Top)} {handle("target", Position.Left)} @@ -103,7 +101,7 @@ const nodeTypes = { {handle("source", Position.Right)} ), - [primerDefNameNodeTypeName]: (p: NodeProps) => ( + "primer-def-name": (p: NodeProps) => ( <>
, + { id: string } & { + [T in keyof typeof nodeTypes]: { + type: T; + data: Parameters<(typeof nodeTypes)[T]>[0]["data"]; + }; + }[keyof typeof nodeTypes] + > +>; + const augmentTree = async ( tree: Tree, p: NodeParams & T @@ -144,7 +156,7 @@ const augmentTree = async ( const [data, nested] = await nodeProps(tree, p); const makeEdge = ( child: PrimerTreeNoPos - ): [PrimerTreeNoPos, Edge] => [ + ): [PrimerTreeNoPos, PrimerEdge] => [ child, { id: JSON.stringify([tree.nodeId, child.node.id]), @@ -162,7 +174,7 @@ const augmentTree = async ( childTrees: childTrees.map((e) => makeEdge(e)), node: { id: tree.nodeId, - type: primerNodeTypeName, + type: "primer", data, }, }, @@ -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 = { + const defNameNode: PrimerNode = { id: defNodeId, data: { def: def.name, @@ -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, Edge]; + subtree: [PrimerTreeNoPos, PrimerEdge]; nested: PrimerGraph[]; }> => { const [t, nested] = await augmentTree(tree, augmentParams); @@ -386,7 +398,7 @@ export const TreeReactFlow = (p: TreeReactFlowProps) => { const id = useId(); return ( - { proOptions={{ hideAttribution: true, account: "paid-pro" }} > - + ); }; @@ -405,7 +417,7 @@ export type TreeReactFlowOneProps = { tree?: Tree; onNodeClick?: ( event: React.MouseEvent, - node: Node> + node: PrimerNode ) => void; } & NodeParams; @@ -439,7 +451,7 @@ export const TreeReactFlowOne = (p: TreeReactFlowOneProps) => { const id = useId(); return ( - { proOptions={{ hideAttribution: true, account: "paid-pro" }} > - + ); }; + +/** 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 = ( + p: Omit[0], "onNodeClick" | "nodes"> & { + nodes: N[]; + onNodeClick?: (e: React.MouseEvent, n: N) => void; + } +): ReturnType => ( + { + "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 + ); + }, + }} + > +); diff --git a/src/components/TreeReactFlow/layoutTree.ts b/src/components/TreeReactFlow/layoutTree.ts index 6e0c5ab8..65bba744 100644 --- a/src/components/TreeReactFlow/layoutTree.ts +++ b/src/components/TreeReactFlow/layoutTree.ts @@ -1,4 +1,3 @@ -import { Node, Edge } from "reactflow"; import { InnerNode as InnerTidyNode, Node as TidyNode, @@ -6,10 +5,9 @@ import { } from "@zxch3n/tidy"; import { WasmLayoutType } from "@zxch3n/tidy/wasm_dist"; import { - Empty, - NodeNoPos, - PrimerDefNameNodeProps, - PrimerNodeProps, + Positioned, + PrimerEdge, + PrimerNode, PrimerTree, PrimerTreeNoPos, treeMap, @@ -57,16 +55,16 @@ export const layoutTree = ( type NodeInfo = { id: number; - node: NodeNoPos | PrimerDefNameNodeProps>; - edges: { edge: Edge; isRight: boolean }[]; + node: PrimerNode; + edges: { edge: PrimerEdge; isRight: boolean }[]; }; // A single node of a `PrimerTree`. // Note that this type is very similar in structure to `PrimerTree`, // the only difference being that this type does not contain the actual subtrees. type PrimerTreeNode = { - node: Node | PrimerDefNameNodeProps>; - edges: Edge[]; - rightEdge?: Edge; + node: Positioned>; + edges: PrimerEdge[]; + rightEdge?: PrimerEdge; }; const makeNodeMap = ( rootId: number, @@ -119,8 +117,8 @@ const primerToTidy = (t: PrimerTreeNoPos): [TidyNode, NodeInfo[]] => { const go = (primerTree: PrimerTreeNoPos): [TidyNode, NodeInfo[]] => { const mkNodeInfos = ( primerTree0: PrimerTreeNoPos[] - ): [TidyNode[], NodeInfo[], Edge[]] => { - const r = primerTree0.map<[TidyNode[], NodeInfo[], Edge[]]>( + ): [TidyNode[], NodeInfo[], PrimerEdge[]] => { + const r = primerTree0.map<[TidyNode[], NodeInfo[], PrimerEdge[]]>( (t) => { const [tree1, nodes1] = go(t); // We explore (transitive) right-children now, diff --git a/src/util.ts b/src/util.ts new file mode 100644 index 00000000..fba35702 --- /dev/null +++ b/src/util.ts @@ -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] 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 = () => {};