refactoring
This commit is contained in:
47
frontend/docs/IMPROVEMENTS.md
Normal file
47
frontend/docs/IMPROVEMENTS.md
Normal file
@@ -0,0 +1,47 @@
|
||||
# Codebase simplification and design patterns
|
||||
|
||||
This doc summarizes recent improvements and suggested next steps for readability, extension, and consistency.
|
||||
|
||||
## Done
|
||||
|
||||
### 1. Single place for template/reachability (DRY)
|
||||
|
||||
- **Added** `lib/graph/templateRefs.ts`: `isReachable`, `resolveExtendsRef`, `getTemplateRefs`.
|
||||
- **Refactored** `config/renderingLogic.ts` and `useRenderingNodeState.ts` to use these helpers instead of duplicating the same logic.
|
||||
- **Pattern:** Extract shared pure helpers into a small lib module; keep call sites thin and consistent.
|
||||
|
||||
### 2. Naming and comments
|
||||
|
||||
- **Renamed** `outputMenuRegistry.tsx` → `outputMenuHandlers.tsx` (no registry, only helpers).
|
||||
- **Updated** `rendering.ts` and `sourceRenderingLogic.ts`: output menu is described as coming from the node descriptor (`getOutputMenuContent`), not a separate registry.
|
||||
- **Documented** `NodeMenubar`: extra content can come from props or from the descriptor (`getNodeMenuExtraContent`).
|
||||
- **Documented** `nodeTypes.ts`: clarifies React Flow types vs node type id (nodeRegistry).
|
||||
|
||||
### 3. Central pipeline entry
|
||||
|
||||
- **rendering.ts** documents the 3-step pipeline, how to add a source/output type, and points to `templateRefs.ts` for shared helpers.
|
||||
|
||||
## Design patterns in use
|
||||
|
||||
| Pattern | Where |
|
||||
|----------------|--------------------------------------------|
|
||||
| **Registry** | nodeRegistry, sourceRenderingLogic |
|
||||
| **Builder** | nodeTypeBuilder (descriptor per node type) |
|
||||
| **Pipeline** | Resolve → Render → Display (rendering.ts) |
|
||||
| **Strategy** | Source logic per node type; output menu per descriptor |
|
||||
| **Shared helpers** | templateRefs, outputMenuHandlers, renderingUtils |
|
||||
|
||||
## Suggested next steps
|
||||
|
||||
1. **CanvasPage** (~950 lines): Split into smaller units, e.g.:
|
||||
- `useCanvasGraph()` or similar for graph state and connection rules.
|
||||
- A dedicated component for the context menu (add node, paste, etc.).
|
||||
- Keeps CanvasPage as composition + layout.
|
||||
|
||||
2. **useRenderingNodeState**: Consider extracting:
|
||||
- Signature building (config/edges/variables/functions/data) into a pure function or small module, e.g. `buildSourceSignatures(nodes, edges, id, incomingIds, getConfigContent)`.
|
||||
- Makes the hook easier to read and the logic testable in isolation.
|
||||
|
||||
3. **Config types**: If you add more output types (e.g. Mermaid), consider a small registry API (`registerConfigType`, `getConfigType`) instead of a single large `CONFIG_TYPES` array, so extensions can register without editing the core list.
|
||||
|
||||
4. **Consistent node shape in lib**: `templateRefs` uses `EdgeLike` / `NodeLike`; other graph code uses inline `{ source, target }` or `nodes as ...`. You could standardize on the same minimal types where appropriate to reduce casts.
|
||||
@@ -1,3 +1,8 @@
|
||||
/**
|
||||
* Per-node menubar (Node, Output, Data, Inputs, Insert). Content can be passed as props
|
||||
* or resolved from the node type descriptor: when nodeMenuExtraContent is not provided,
|
||||
* it is taken from getNodeType(nodeType)?.getNodeMenuExtraContent?.(nodeId, data).
|
||||
*/
|
||||
import React, { useCallback, useContext, useMemo } from 'react'
|
||||
import FlowContext from '@/lib/graph/flowContext'
|
||||
import { getNodeType } from '@/lib/graph/nodeRegistry'
|
||||
|
||||
@@ -13,7 +13,7 @@ import {
|
||||
import { getResolvedContentForConfig } from './renderingLogic'
|
||||
import type { NodeTypeDescriptor } from '@/lib/graph/nodeRegistry'
|
||||
import ConfigNode from './ConfigNode'
|
||||
import { createImageExportHandlers, type OutputMenuContext } from '@/components/nodes/render/outputMenuRegistry'
|
||||
import { createImageExportHandlers, type OutputMenuContext } from '@/components/nodes/render/outputMenuHandlers'
|
||||
|
||||
const ICON_CLASS = 'mr-2 h-4 w-4'
|
||||
|
||||
|
||||
@@ -7,43 +7,11 @@
|
||||
import nunjucks from 'nunjucks'
|
||||
import type { ResolvedContentResult, SourceRenderingLogicContext } from '@/lib/graph/rendering'
|
||||
import { getConfigContent, getConfigType, getConfigTypeId, type ConfigTypeId } from '@/lib/graph/rendering'
|
||||
import { isReachable, resolveExtendsRef, getTemplateRefs } from '@/lib/graph/templateRefs'
|
||||
|
||||
type Node = { id: string; type?: string; data?: unknown }
|
||||
type Edge = { id: string; source: string; target: string }
|
||||
|
||||
function isReachable(edges: Edge[], startId: string, targetId: string): boolean {
|
||||
const q: string[] = [startId]
|
||||
const seen = new Set<string>([startId])
|
||||
while (q.length) {
|
||||
const cur = q.shift()!
|
||||
if (cur === targetId) return true
|
||||
for (const e of edges) {
|
||||
if (e.source === cur && !seen.has(e.target)) {
|
||||
seen.add(e.target)
|
||||
q.push(e.target)
|
||||
}
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
function resolveExtendsRef(nodes: Node[], name: string): string {
|
||||
const refName = name.replace(/\.(puml|html)$/, '').trim()
|
||||
return nodes.find((n) => n.id === refName || (n.data as Record<string, unknown>)?.title === refName)?.id ?? refName
|
||||
}
|
||||
|
||||
function getTemplateRefs(content: string): string[] {
|
||||
const refs: string[] = []
|
||||
const extendMatch = content.match(/\{\%\s*extends\s+["']([^"']+)["']\s*\%\}/)
|
||||
if (extendMatch) refs.push(extendMatch[1].trim())
|
||||
const includeRegex = /\{\%\s*include\s+["']([^"']+)["']\s*\%\}/g
|
||||
let m
|
||||
while ((m = includeRegex.exec(content)) !== null) refs.push(m[1].trim())
|
||||
const importRegex = /\{\%\s*import\s+["']([^"']+)["']\s+as\s+\w+\s*\%\}/g
|
||||
while ((m = importRegex.exec(content)) !== null) refs.push(m[1].trim())
|
||||
return refs
|
||||
}
|
||||
|
||||
export async function getResolvedContentForConfig(context: SourceRenderingLogicContext): Promise<ResolvedContentResult> {
|
||||
const { nodes, edges, sourceNodeId, renderNodeId } = context
|
||||
const srcId = sourceNodeId
|
||||
@@ -56,11 +24,11 @@ export async function getResolvedContentForConfig(context: SourceRenderingLogicC
|
||||
const configIdsUsed = new Set<string>()
|
||||
|
||||
const addConfigAndRefs = (templateName: string, visited = new Set<string>()) => {
|
||||
const refId = resolveExtendsRef(nodes, templateName)
|
||||
const refId = resolveExtendsRef(nodes as { id: string; data?: unknown }[], templateName)
|
||||
if (visited.has(refId)) throw new Error(`Circular reference detected: ${templateName}`)
|
||||
const node = nodes.find((n) => n.id === refId && n.type === 'config')
|
||||
if (!node) throw new Error(`Config not found: ${templateName}`)
|
||||
if (refId !== srcId && !isReachable(edges, refId, id))
|
||||
if (refId !== srcId && !isReachable(edges as { source: string; target: string }[], refId, id))
|
||||
throw new Error(`Referenced config not connected to renderer: ${templateName}`)
|
||||
visited.add(refId)
|
||||
configIdsUsed.add(refId)
|
||||
@@ -72,10 +40,10 @@ export async function getResolvedContentForConfig(context: SourceRenderingLogicC
|
||||
|
||||
const configLoader = {
|
||||
getSource: (name: string): { src: string; path: string } | null => {
|
||||
const refId = resolveExtendsRef(nodes, name)
|
||||
const refId = resolveExtendsRef(nodes as { id: string; data?: unknown }[], name)
|
||||
const node = nodes.find((n) => n.id === refId && n.type === 'config')
|
||||
if (!node) return null
|
||||
if (refId !== srcId && !isReachable(edges, refId, id))
|
||||
if (refId !== srcId && !isReachable(edges as { source: string; target: string }[], refId, id))
|
||||
throw new Error(`Referenced config not connected to renderer: ${name}`)
|
||||
return {
|
||||
src: getConfigContent((node.data ?? undefined) as Record<string, unknown> | undefined),
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
export { default as RenderingNode, type RenderingNodeData } from './RenderingNode'
|
||||
export { getRenderNodeDescriptor } from './descriptor'
|
||||
export { createImageExportHandlers } from './outputMenuRegistry'
|
||||
export type { OutputMenuContext } from './outputMenuRegistry'
|
||||
export { createImageExportHandlers } from './outputMenuHandlers'
|
||||
export type { OutputMenuContext } from './outputMenuHandlers'
|
||||
|
||||
@@ -20,6 +20,7 @@ import {
|
||||
processSvgDisplay,
|
||||
stripTemplateSyntax,
|
||||
} from '@/lib/graph/rendering'
|
||||
import { isReachable, resolveExtendsRef, getTemplateRefs } from '@/lib/graph/templateRefs'
|
||||
import type { ConfigTypeId, SourceRenderingLogicContext } from '@/lib/graph/rendering'
|
||||
|
||||
export type RenderingNodeData = {
|
||||
@@ -136,54 +137,22 @@ export function useRenderingNodeState(
|
||||
: ''
|
||||
|
||||
const connectedNodeIds = useMemo(() => {
|
||||
const edgeList = edges as { source: string; target: string }[]
|
||||
const nodeList = nodes as { id: string; type?: string; data?: unknown }[]
|
||||
const out = new Set<string>()
|
||||
const isReachable = (startId: string, targetId: string) => {
|
||||
const q: string[] = [startId]
|
||||
const seen = new Set<string>([startId])
|
||||
while (q.length) {
|
||||
const cur = q.shift()!
|
||||
if (cur === targetId) return true
|
||||
for (const e of edges as { source: string; target: string }[]) {
|
||||
if (e.source === cur && !seen.has(e.target)) {
|
||||
seen.add(e.target)
|
||||
q.push(e.target)
|
||||
}
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
const resolveRef = (name: string) => {
|
||||
const refName = name.replace(/\.(puml|html)$/, '').trim()
|
||||
return (nodes as { id: string; data?: { title?: string } }[]).find(
|
||||
(n) => n.id === refName || n.data?.title === refName
|
||||
)?.id ?? refName
|
||||
}
|
||||
const getTemplateRefs = (content: string): string[] => {
|
||||
const refs: string[] = []
|
||||
const extendMatch = content.match(/\{\%\s*extends\s+["']([^"']+)["']\s*\%\}/)
|
||||
if (extendMatch) refs.push(extendMatch[1].trim())
|
||||
const includeRegex = /\{\%\s*include\s+["']([^"']+)["']\s*\%\}/g
|
||||
let m
|
||||
while ((m = includeRegex.exec(content)) !== null) refs.push(m[1].trim())
|
||||
const importRegex = /\{\%\s*import\s+["']([^"']+)["']\s+as\s+\w+\s*\%\}/g
|
||||
while ((m = importRegex.exec(content)) !== null) refs.push(m[1].trim())
|
||||
return refs
|
||||
}
|
||||
const addConfigRefs = (nodeId: string, visited: Set<string>) => {
|
||||
if (visited.has(nodeId)) return
|
||||
const node = (nodes as { id: string; type?: string; data?: unknown }[]).find(
|
||||
(n) => n.id === nodeId && n.type === 'config'
|
||||
)
|
||||
const node = nodeList.find((n) => n.id === nodeId && n.type === 'config')
|
||||
if (!node) return
|
||||
visited.add(nodeId)
|
||||
out.add(nodeId)
|
||||
const content = getConfigContent((node.data ?? undefined) as Record<string, unknown> | undefined)
|
||||
for (const ref of getTemplateRefs(content)) {
|
||||
const refId = resolveRef(ref)
|
||||
const refId = resolveExtendsRef(nodeList, ref)
|
||||
if (
|
||||
refId &&
|
||||
(nodes as { id: string; type?: string }[]).some((n) => n.id === refId && n.type === 'config') &&
|
||||
isReachable(refId, id)
|
||||
nodeList.some((n) => n.id === refId && n.type === 'config') &&
|
||||
isReachable(edgeList, refId, id)
|
||||
) {
|
||||
addConfigRefs(refId, visited)
|
||||
}
|
||||
@@ -191,11 +160,11 @@ export function useRenderingNodeState(
|
||||
}
|
||||
const configVisited = new Set<string>()
|
||||
for (const nid of incomingIds) {
|
||||
const node = (nodes as { id: string; type?: string }[]).find((n) => n.id === nid)
|
||||
const node = nodeList.find((n) => n.id === nid)
|
||||
if (node?.type === 'config') addConfigRefs(nid, configVisited)
|
||||
else out.add(nid)
|
||||
}
|
||||
for (const e of edges as { source: string; target: string }[]) {
|
||||
for (const e of edgeList) {
|
||||
if (out.has(e.target)) out.add(e.source)
|
||||
}
|
||||
return out
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
/**
|
||||
* Central node and edge types for the app. Use AppNode / AppEdge in graph state and context.
|
||||
* React Flow node/edge types for the app (graph state and context). AppNode / AppEdge
|
||||
* describe the shape of nodes and edges; the node type *id* (e.g. 'config', 'render')
|
||||
* lives in nodeRegistry and is stored as node.type.
|
||||
*/
|
||||
|
||||
import type { Node, Edge } from '@xyflow/react'
|
||||
|
||||
@@ -13,15 +13,20 @@
|
||||
*
|
||||
* 3. **Display** — The Rendering node picks a view (image viewport vs markdown content)
|
||||
* from `outputType` on the renderer and shows the result. Output menu items (e.g. Export)
|
||||
* come from the source/renderer via `outputMenuDescriptor`.
|
||||
* come from the source node descriptor's getOutputMenuContent (see NodeTypeDescriptor).
|
||||
*
|
||||
* ## Implementing a new source
|
||||
* - Implement `IRenderingSource` (resolve + optional output menu).
|
||||
* - Register with `registerSourceRenderingLogic(nodeType, logic)`.
|
||||
* - Implement source rendering logic (resolve step) and set it on the node descriptor
|
||||
* via .sourceRenderingLogic(). Optionally set .outputMenuContent() for the Output menu.
|
||||
* - The descriptor is registered via registerNodeType(); source logic is registered automatically.
|
||||
*
|
||||
* ## Implementing a new output type
|
||||
* - Add a config type (or equivalent) fulfilling `IOutputTypeRenderer`.
|
||||
* - Register in CONFIG_TYPES and use `getConfigType(id)` in the resolve step.
|
||||
*
|
||||
* ## Shared helpers
|
||||
* - templateRefs.ts: isReachable, resolveExtendsRef, getTemplateRefs (Nunjucks extends/include/import).
|
||||
* Used by config resolve and the rendering node hook so template/reachability logic stays in one place.
|
||||
*/
|
||||
|
||||
import type {
|
||||
@@ -60,8 +65,7 @@ export interface IResolveResult {
|
||||
|
||||
/**
|
||||
* Contract for a node type that can feed the Rendering node (config, agent, etc.).
|
||||
* Register via registerSourceRenderingLogic(nodeType, logic).
|
||||
* Output menu content is registered per output type in the frontend (outputMenuRegistry).
|
||||
* Set on the node descriptor via .sourceRenderingLogic(); output menu via .outputMenuContent().
|
||||
*/
|
||||
export interface IRenderingSource {
|
||||
/** When to re-run: 'auto' on upstream changes, 'manual' only on Run. */
|
||||
|
||||
@@ -30,7 +30,7 @@ export type ResolvedContentResult = {
|
||||
reasoning?: string
|
||||
}
|
||||
|
||||
/** Source logic implementation. Implements {@link IRenderingSource}. Output menu content is provided per output type via the frontend registry (see outputMenuRegistry). */
|
||||
/** Source logic implementation. Implements {@link IRenderingSource}. Output menu is provided by the node descriptor's getOutputMenuContent. */
|
||||
export type SourceRenderingLogic = {
|
||||
defaultUpdateMode: 'auto' | 'manual'
|
||||
getResolvedContent: (context: SourceRenderingLogicContext) => Promise<ResolvedContentResult>
|
||||
|
||||
54
frontend/src/lib/graph/templateRefs.ts
Normal file
54
frontend/src/lib/graph/templateRefs.ts
Normal file
@@ -0,0 +1,54 @@
|
||||
/**
|
||||
* Shared helpers for Nunjucks template references (extends/include/import) and
|
||||
* graph reachability. Used by config resolve logic and the rendering node hook
|
||||
* so template parsing and "connected configs" stay in one place.
|
||||
*/
|
||||
|
||||
export type EdgeLike = { source: string; target: string }
|
||||
export type NodeLike = { id: string; data?: unknown }
|
||||
|
||||
/** BFS: is target reachable from start following directed edges? */
|
||||
export function isReachable(
|
||||
edges: EdgeLike[],
|
||||
startId: string,
|
||||
targetId: string
|
||||
): boolean {
|
||||
const q: string[] = [startId]
|
||||
const seen = new Set<string>([startId])
|
||||
while (q.length) {
|
||||
const cur = q.shift()!
|
||||
if (cur === targetId) return true
|
||||
for (const e of edges) {
|
||||
if (e.source === cur && !seen.has(e.target)) {
|
||||
seen.add(e.target)
|
||||
q.push(e.target)
|
||||
}
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
/** Resolve a template name to a node id (by id or by data.title). */
|
||||
export function resolveExtendsRef(nodes: NodeLike[], name: string): string {
|
||||
const refName = name.replace(/\.(puml|html)$/, '').trim()
|
||||
return (
|
||||
nodes.find(
|
||||
(n) =>
|
||||
n.id === refName ||
|
||||
(n.data as Record<string, unknown>)?.title === refName
|
||||
)?.id ?? refName
|
||||
)
|
||||
}
|
||||
|
||||
/** Extract template refs from content: extends "x", include "y", import "z" as ... */
|
||||
export function getTemplateRefs(content: string): string[] {
|
||||
const refs: string[] = []
|
||||
const extendMatch = content.match(/\{\%\s*extends\s+["']([^"']+)["']\s*\%\}/)
|
||||
if (extendMatch) refs.push(extendMatch[1].trim())
|
||||
const includeRegex = /\{\%\s*include\s+["']([^"']+)["']\s*\%\}/g
|
||||
let m
|
||||
while ((m = includeRegex.exec(content)) !== null) refs.push(m[1].trim())
|
||||
const importRegex = /\{\%\s*import\s+["']([^"']+)["']\s+as\s+\w+\s*\%\}/g
|
||||
while ((m = importRegex.exec(content)) !== null) refs.push(m[1].trim())
|
||||
return refs
|
||||
}
|
||||
Reference in New Issue
Block a user