Skip to content

Commit 0e0121f

Browse files
DavidBabinecDavidBabinec
authored
refactor(page-tree): reuse traversal selectors in form analysis (CoreBunch#81)
1 parent c5c9686 commit 0e0121f

4 files changed

Lines changed: 59 additions & 73 deletions

File tree

‎src/__tests__/architecture/page-tree-single-source.test.ts‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
import { describe, expect, test } from 'bun:test'
2+
import { readFileSync } from 'fs'
3+
import { join } from 'path'
4+
5+
const PROJECT_ROOT = join(import.meta.dir, '../../../')
6+
7+
const FORM_TREE_CONSUMERS = [
8+
'src/core/forms/snapshot.ts',
9+
'src/admin/pages/site/panels/PropertiesPanel/formSettingsAnalysis.ts',
10+
]
11+
12+
function source(relativePath: string): string {
13+
return readFileSync(join(PROJECT_ROOT, relativePath), 'utf8')
14+
}
15+
16+
describe('Page-tree selector source of truth', () => {
17+
test('form analysis uses @core/page-tree traversal helpers instead of local tree walkers', () => {
18+
for (const file of FORM_TREE_CONSUMERS) {
19+
const content = source(file)
20+
expect(content, `${file} must not define a local recursive tree walker`).not.toMatch(
21+
/\bfunction\s+walkTree\s*\(/,
22+
)
23+
expect(content, `${file} must not derive a parent map from children arrays`).not.toMatch(
24+
/\bfunction\s+buildParentMap\s*\(/,
25+
)
26+
}
27+
})
28+
})

‎src/__tests__/forms/formSettingsAnalysis.test.ts‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { describe, expect, it } from 'bun:test'
22
import type { DataTable } from '@core/data/schemas'
33
import type { Page, PageNode } from '@core/page-tree'
4+
import { reindexNodeParents } from '@core/page-tree'
45
import {
56
analyzeFormSettings,
67
buildDataTableDraftFromForm,
@@ -46,7 +47,7 @@ const table: DataTable = {
4647
}
4748

4849
function makePage(): Page {
49-
return {
50+
return withParentIndex({
5051
id: 'page-home',
5152
slug: 'index',
5253
title: 'Home',
@@ -74,7 +75,12 @@ function makePage(): Page {
7475
submit: node('submit', 'base.submit', { label: 'Send', formId: '' }),
7576
'outside-input': node('outside-input', 'base.input', { fieldId: 'email', name: 'email' }),
7677
},
77-
}
78+
})
79+
}
80+
81+
function withParentIndex(page: Page): Page {
82+
reindexNodeParents(page.nodes)
83+
return page
7884
}
7985

8086
describe('analyzeFormSettings', () => {
@@ -144,7 +150,7 @@ describe('analyzeFormSettings', () => {
144150
})
145151

146152
it('infers a create-table draft from controls inside the selected form', () => {
147-
const page: Page = {
153+
const page: Page = withParentIndex({
148154
id: 'page-home',
149155
slug: 'index',
150156
title: 'Home',
@@ -174,7 +180,7 @@ describe('analyzeFormSettings', () => {
174180
'label-consent': node('label-consent', 'base.label', { text: 'Consent', targetMode: 'auto', targetId: '' }),
175181
consent: node('consent', 'base.checkbox', { name: 'consent', required: true }),
176182
},
177-
}
183+
})
178184

179185
const analysis = analyzeFormSettings({ page, nodeId: 'form' })
180186
expect(analysis.inferredFields).toEqual([

‎src/admin/pages/site/panels/PropertiesPanel/formSettingsAnalysis.ts‎

Lines changed: 16 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import type { CreateDataTableInput, DataField, DataTable, DataSelectOption } from '@core/data/schemas'
22
import type { ImportFragment } from '@core/htmlImport'
3-
import type { Page, PageNode } from '@core/page-tree'
4-
import { createNode } from '@core/page-tree'
3+
import { createNode, flattenSubtree, getParent, type Page, type PageNode } from '@core/page-tree'
54
import { formDisplayName, humanizeIdentifier, slugifyFormTableName } from './formSettingsNaming'
65

76
export { formDisplayName } from './formSettingsNaming'
@@ -111,12 +110,11 @@ export function analyzeFormSettings(input: {
111110
return emptyAnalysis(selectedNode, table)
112111
}
113112

114-
const parentByNodeId = buildParentMap(page)
115113
const formNode = selectedNode.moduleId === 'base.form'
116114
? selectedNode
117-
: nearestAncestorForm(page, selectedNode.id, parentByNodeId)
115+
: nearestAncestorForm(page, selectedNode.id)
118116
const form = formNode ? formSummary(formNode) : null
119-
const inferredFields = formNode ? inferFieldsFromForm(page, formNode, parentByNodeId) : []
117+
const inferredFields = formNode ? inferFieldsFromForm(page, formNode) : []
120118
const missingFields = table && formNode ? fieldsMissingFromForm(page, formNode, table) : []
121119
const kind = settingsKind(selectedNode.moduleId)
122120
const warnings: FormSettingsWarning[] = []
@@ -171,7 +169,7 @@ export function analyzeFormSettings(input: {
171169

172170
let inferredTarget: FormTargetSummary | null = null
173171
if (kind === 'label') {
174-
inferredTarget = inferLabelTarget(page, selectedNode, parentByNodeId)
172+
inferredTarget = inferLabelTarget(page, selectedNode)
175173
if (!inferredTarget) {
176174
warnings.push({
177175
code: 'label_without_target',
@@ -351,7 +349,7 @@ function formSummary(node: PageNode): FormContextSummary {
351349
function duplicateNameWarnings(page: Page, formNode: PageNode): FormSettingsWarning[] {
352350
const seen = new Map<string, string>()
353351
const warnings: FormSettingsWarning[] = []
354-
for (const nodeId of walkTree(page, formNode.id)) {
352+
for (const nodeId of flattenSubtree(page, formNode.id)) {
355353
if (nodeId === formNode.id) continue
356354
const node = page.nodes[nodeId]
357355
if (!node || !FORM_CONTROL_MODULES.has(node.moduleId)) continue
@@ -373,15 +371,14 @@ function duplicateNameWarnings(page: Page, formNode: PageNode): FormSettingsWarn
373371
function inferFieldsFromForm(
374372
page: Page,
375373
formNode: PageNode,
376-
parentByNodeId: Map<string, string>,
377374
): DataField[] {
378375
const fields: DataField[] = []
379376
const usedIds = new Set<string>()
380-
for (const nodeId of walkTree(page, formNode.id)) {
377+
for (const nodeId of flattenSubtree(page, formNode.id)) {
381378
if (nodeId === formNode.id) continue
382379
const node = page.nodes[nodeId]
383380
if (!node || !FORM_CONTROL_MODULES.has(node.moduleId)) continue
384-
const field = inferFieldFromControl(page, node, parentByNodeId, usedIds)
381+
const field = inferFieldFromControl(page, node, usedIds)
385382
if (field) fields.push(field)
386383
}
387384
return fields
@@ -390,10 +387,9 @@ function inferFieldsFromForm(
390387
function inferFieldFromControl(
391388
page: Page,
392389
node: PageNode,
393-
parentByNodeId: Map<string, string>,
394390
usedIds: Set<string>,
395391
): DataField | null {
396-
const label = labelForControl(page, node, parentByNodeId)
392+
const label = labelForControl(page, node)
397393
const rawId = stringProp(node, 'fieldId', '')
398394
|| stringProp(node, 'name', '')
399395
|| stringProp(node, 'id', '')
@@ -462,7 +458,7 @@ function inferFieldFromControl(
462458

463459
function fieldsMissingFromForm(page: Page, formNode: PageNode, table: DataTable): DataField[] {
464460
const representedFieldIds = new Set<string>()
465-
for (const nodeId of walkTree(page, formNode.id)) {
461+
for (const nodeId of flattenSubtree(page, formNode.id)) {
466462
if (nodeId === formNode.id) continue
467463
const node = page.nodes[nodeId]
468464
if (!node || !FORM_CONTROL_MODULES.has(node.moduleId)) continue
@@ -475,10 +471,8 @@ function fieldsMissingFromForm(page: Page, formNode: PageNode, table: DataTable)
475471
function labelForControl(
476472
page: Page,
477473
node: PageNode,
478-
parentByNodeId: Map<string, string>,
479474
): string {
480-
const parentId = parentByNodeId.get(node.id)
481-
const parent = parentId ? page.nodes[parentId] : null
475+
const parent = getParent(page, node.id)
482476
if (parent) {
483477
const nodeIndex = parent.children.indexOf(node.id)
484478
for (const siblingId of parent.children.slice(0, nodeIndex).reverse()) {
@@ -510,7 +504,6 @@ function optionFieldsFromSelect(page: Page, selectNode: PageNode): DataSelectOpt
510504
function inferLabelTarget(
511505
page: Page,
512506
labelNode: PageNode,
513-
parentByNodeId: Map<string, string>,
514507
): FormTargetSummary | null {
515508
const explicit = stringProp(labelNode, 'targetId', '')
516509
if (stringProp(labelNode, 'targetMode', 'auto') === 'explicit' && explicit) {
@@ -520,12 +513,11 @@ function inferLabelTarget(
520513
: { nodeId: explicit, label: explicit }
521514
}
522515

523-
const parentId = parentByNodeId.get(labelNode.id)
524-
const parent = parentId ? page.nodes[parentId] : null
516+
const parent = getParent(page, labelNode.id)
525517
if (!parent) return null
526518
const labelIndex = parent.children.indexOf(labelNode.id)
527519
for (const siblingId of parent.children.slice(labelIndex + 1)) {
528-
for (const candidateId of walkTree(page, siblingId)) {
520+
for (const candidateId of flattenSubtree(page, siblingId)) {
529521
const candidate = page.nodes[candidateId]
530522
if (candidate && FORM_CONTROL_MODULES.has(candidate.moduleId)) {
531523
return { nodeId: candidate.id, label: controlLabel(candidate) }
@@ -535,41 +527,15 @@ function inferLabelTarget(
535527
return null
536528
}
537529

538-
function nearestAncestorForm(
539-
page: Page,
540-
nodeId: string,
541-
parentByNodeId: Map<string, string>,
542-
): PageNode | null {
543-
let currentId = parentByNodeId.get(nodeId)
544-
while (currentId) {
545-
const current = page.nodes[currentId]
546-
if (!current) return null
530+
function nearestAncestorForm(page: Page, nodeId: string): PageNode | null {
531+
let current = getParent(page, nodeId)
532+
while (current) {
547533
if (current.moduleId === 'base.form') return current
548-
currentId = parentByNodeId.get(current.id)
534+
current = getParent(page, current.id)
549535
}
550536
return null
551537
}
552538

553-
function buildParentMap(page: Page): Map<string, string> {
554-
const map = new Map<string, string>()
555-
for (const node of Object.values(page.nodes)) {
556-
for (const childId of node.children) map.set(childId, node.id)
557-
}
558-
return map
559-
}
560-
561-
function walkTree(page: Page, startNodeId: string): string[] {
562-
const out: string[] = []
563-
const visit = (nodeId: string) => {
564-
const node = page.nodes[nodeId]
565-
if (!node) return
566-
out.push(nodeId)
567-
for (const childId of node.children) visit(childId)
568-
}
569-
visit(startNodeId)
570-
return out
571-
}
572-
573539
function stringProp(node: PageNode, key: string, fallback: string): string {
574540
const value = node.props[key]
575541
return typeof value === 'string' ? value : fallback

‎src/core/forms/snapshot.ts‎

Lines changed: 5 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type { Page, PageNode } from '@core/page-tree'
1+
import { flattenSubtree, getParent, type Page, type PageNode } from '@core/page-tree'
22
import { normalizeIdentifierValue } from '@core/utils/identifier'
33
import type {
44
FormControlBinding,
@@ -19,7 +19,7 @@ const FORM_CONTROL_MODULES = new Set([
1919
export function derivePageFormSnapshots(page: Page): PublishedFormSnapshot[] {
2020
const snapshots: PublishedFormSnapshot[] = []
2121

22-
for (const nodeId of walkTree(page, page.rootNodeId)) {
22+
for (const nodeId of flattenSubtree(page, page.rootNodeId)) {
2323
const node = page.nodes[nodeId]
2424
if (!node || node.moduleId !== 'base.form') continue
2525
const mode = stringProp(node, 'mode', 'cms')
@@ -36,7 +36,7 @@ function deriveFormSnapshot(
3636
): PublishedFormSnapshot {
3737
const fallbackFormId = normalizeIdentifierValue(formNode.id, 'form')
3838
const formId = normalizeIdentifierValue(stringProp(formNode, 'formId', formNode.id), fallbackFormId)
39-
const descendantIds = walkTree(page, formNode.id).filter((nodeId) => nodeId !== formNode.id)
39+
const descendantIds = flattenSubtree(page, formNode.id).filter((nodeId) => nodeId !== formNode.id)
4040
const controls: FormControlBinding[] = []
4141
const labels: PublishedFormLabel[] = []
4242
const submits: PublishedFormSubmit[] = []
@@ -131,14 +131,12 @@ function inferLabelTarget(
131131
return target?.id ?? explicit
132132
}
133133

134-
const parentId = page.nodes[labelNode.id]?.parentId
135-
if (!parentId) return null
136-
const parent = page.nodes[parentId]
134+
const parent = getParent(page, labelNode.id)
137135
if (!parent) return null
138136
const labelIndex = parent.children.indexOf(labelNode.id)
139137
const siblingIds = parent.children.slice(labelIndex + 1)
140138
for (const siblingId of siblingIds) {
141-
for (const candidateId of walkTree(page, siblingId)) {
139+
for (const candidateId of flattenSubtree(page, siblingId)) {
142140
if (candidateId === formNodeId) continue
143141
const candidate = page.nodes[candidateId]
144142
if (candidate && FORM_CONTROL_MODULES.has(candidate.moduleId)) return candidate.id
@@ -147,18 +145,6 @@ function inferLabelTarget(
147145
return null
148146
}
149147

150-
function walkTree(page: Page, startNodeId: string): string[] {
151-
const out: string[] = []
152-
const visit = (nodeId: string) => {
153-
const node = page.nodes[nodeId]
154-
if (!node) return
155-
out.push(nodeId)
156-
for (const childId of node.children) visit(childId)
157-
}
158-
visit(startNodeId)
159-
return out
160-
}
161-
162148
function stringProp(node: PageNode, key: string, fallback: string): string {
163149
const value = node.props[key]
164150
return typeof value === 'string' ? value : fallback

0 commit comments

Comments
 (0)