From 9eaea85473913993868ebe637a919bee0f2913db Mon Sep 17 00:00:00 2001 From: Miles Malerba Date: Wed, 30 Apr 2025 22:12:42 +0000 Subject: [PATCH 1/4] refactor: add a new version of the logic node Adds a new version of logic node that is created via builders. The builder / actual logic node split allows the final logic for a node to be lazily merged together. My plan is to eventually swap this in for FieldLogicNode and have the schema methods (`validate`, `apply`, etc.) work with the builders. The FieldNode will then build its logic when it is first created. This should be the other main piece aside from relative paths which is needed to make self-referential logic work. --- packages/forms/experimental/src/logic_node.ts | 6 +- .../forms/experimental/src/logic_node_2.ts | 230 +++++++++++++++ .../experimental/test/logic_node_2.spec.ts | 279 ++++++++++++++++++ 3 files changed, 512 insertions(+), 3 deletions(-) create mode 100644 packages/forms/experimental/src/logic_node_2.ts create mode 100644 packages/forms/experimental/test/logic_node_2.spec.ts diff --git a/packages/forms/experimental/src/logic_node.ts b/packages/forms/experimental/src/logic_node.ts index da1a0a1ee259..a4181d0cd2cb 100644 --- a/packages/forms/experimental/src/logic_node.ts +++ b/packages/forms/experimental/src/logic_node.ts @@ -113,7 +113,7 @@ export abstract class AbstractLogic { } } -class BooleanOrLogic extends AbstractLogic { +export class BooleanOrLogic extends AbstractLogic { override get defaultValue() { return false; } @@ -123,7 +123,7 @@ class BooleanOrLogic extends AbstractLogic { } } -class ArrayMergeLogic extends AbstractLogic< +export class ArrayMergeLogic extends AbstractLogic< TElement[], TElement | TElement[] | undefined > { @@ -146,7 +146,7 @@ class ArrayMergeLogic extends AbstractLogic< } } -class MetadataMergeLogic extends AbstractLogic { +export class MetadataMergeLogic extends AbstractLogic { override get defaultValue() { return this.key.defaultValue; } diff --git a/packages/forms/experimental/src/logic_node_2.ts b/packages/forms/experimental/src/logic_node_2.ts new file mode 100644 index 000000000000..d8f487d16c59 --- /dev/null +++ b/packages/forms/experimental/src/logic_node_2.ts @@ -0,0 +1,230 @@ +import {FieldContext, FormError, LogicFn, MetadataKey, ValidationResult} from '../public_api'; +import { + AbstractLogic, + ArrayMergeLogic, + BooleanOrLogic, + MetadataMergeLogic, + Predicate, +} from './logic_node'; + +/** + * Container for all the different types of logic that can be applied to a field + * (disabled, hidden, errors, etc.) + */ +export class Logic { + readonly hidden: BooleanOrLogic; + readonly disabled: BooleanOrLogic; + readonly errors: ArrayMergeLogic; + private readonly metadata = new Map, AbstractLogic>(); + + constructor(private predicate: Predicate | undefined) { + this.hidden = new BooleanOrLogic(predicate); + this.disabled = new BooleanOrLogic(predicate); + this.errors = new ArrayMergeLogic(predicate); + } + + getMetadata(key: MetadataKey): AbstractLogic { + if (!this.metadata.has(key as MetadataKey)) { + this.metadata.set(key as MetadataKey, new MetadataMergeLogic(this.predicate, key)); + } + return this.metadata.get(key as MetadataKey)! as AbstractLogic; + } + + readMetadata(key: MetadataKey, arg: FieldContext): T { + if (this.metadata.has(key as MetadataKey)) { + return this.metadata.get(key as MetadataKey)!.compute(arg) as T; + } else { + return key.defaultValue; + } + } + + getMetadataKeys() { + return this.metadata.keys(); + } +} + +/** + * A tree structure of `Logic` corresponding to a tree of fields. + */ +export class LogicNode { + readonly logic: Logic; + + constructor(private builder: LogicNodeBuilder | undefined) { + this.logic = builder ? createLogic(builder) : new Logic(undefined); + } + + getChild(key: PropertyKey): LogicNode { + // The logic for a particular child may be spread across multiple builders. We lazily combine + // this logic at the time the child logic node is requested to be created. + const childBuilders = this.builder ? getAllChildren(this.builder, key) : []; + if (childBuilders.length === 0) { + return new LogicNode(undefined); + } else { + // If there are child builders, merge them under a new builder to ensure the predicate is + // propagated. + const combined = LogicNodeBuilder.newRoot(this.builder?.predicate); + for (const child of childBuilders) { + combined.mergeIn(child); + } + return new LogicNode(combined); + } + } +} + +/** + * A builder for `LogicNode`, which itself is a tree. + */ +export abstract class LogicNodeBuilder { + protected constructor(readonly predicate: Predicate | undefined) {} + + abstract addHiddenRule(logic: LogicFn): void; + abstract addDisabledRule(logic: LogicFn): void; + abstract addErrorRule(logic: LogicFn): void; + abstract addMetadataRule(key: MetadataKey, logic: LogicFn): void; + abstract getChild(key: PropertyKey): MergableLogicNodeBuilder; + + build(): LogicNode { + return new LogicNode(this); + } + + static newRoot(predicate: Predicate | undefined): MergableLogicNodeBuilder { + return new CompositeLogicNodeBuilder(predicate); + } +} + +/** + * A subclass of `LogicNodeBuilder` that supports merging with other logic trees. + */ +export interface MergableLogicNodeBuilder extends LogicNodeBuilder { + mergeIn(other: MergableLogicNodeBuilder): void; +} + +/** + * A `MergableLogicNodeBuilder` that delegates to its "current" builder and creates a new current + * builder after another builder tree is merged in. + */ +class CompositeLogicNodeBuilder extends LogicNodeBuilder implements MergableLogicNodeBuilder { + private current: SimpleLogicNodeBuilder | undefined; + readonly all: LogicNodeBuilder[] = []; + + override addHiddenRule(logic: LogicFn): void { + this.getCurrent().addHiddenRule(logic); + } + + override addDisabledRule(logic: LogicFn): void { + this.getCurrent().addDisabledRule(logic); + } + + override addErrorRule(logic: LogicFn): void { + this.getCurrent().addErrorRule(logic); + } + + override addMetadataRule(key: MetadataKey, logic: LogicFn): void { + this.getCurrent().addMetadataRule(key, logic); + } + + override getChild(key: PropertyKey): MergableLogicNodeBuilder { + return this.getCurrent().getChild(key); + } + + mergeIn(other: MergableLogicNodeBuilder): void { + // Add the other builder to our collection, we'll defer the actual merging of the logic until + // the logic node is requested to be created. In order to preserve the original ordering of the + // rules, we close off the current builder to any further edits. If additional logic is added, + // a new current builder will be created to capture it. + this.all.push(other); + this.current = undefined; + } + + private getCurrent(): SimpleLogicNodeBuilder { + // All rules added to this builder get added on to the current builder. If there is no current + // builder, a new one is created. In order to preserve the original ordering of the rules, we + // clear the current builder whenever a separate builder tree is merged in. + if (this.current === undefined) { + this.current = new SimpleLogicNodeBuilder(this.predicate); + this.all.push(this.current); + } + return this.current; + } +} + +/** + * A `LogicNodeBuilder` that directly adds logic to its backing `Logic` instance. + * + * The user should not be given a reference to this class, it is used internally to keep track of + * concrete tree chunks within the `CompositeLogicNodeBuilder`. When handing a `LogicNodeBuilder` + * to the user it should always be a `CompositeLogicNodeBuilder`. + */ +class SimpleLogicNodeBuilder extends LogicNodeBuilder { + readonly logic = new Logic(undefined); + readonly children = new Map(); + + override addHiddenRule(logic: LogicFn): void { + this.logic.hidden.push(logic); + } + + override addDisabledRule(logic: LogicFn): void { + this.logic.disabled.push(logic); + } + + override addErrorRule(logic: LogicFn): void { + this.logic.errors.push(logic); + } + + override addMetadataRule(key: MetadataKey, logic: LogicFn): void { + this.logic.getMetadata(key).push(logic); + } + + override getChild(key: PropertyKey): MergableLogicNodeBuilder { + // We always create a `CompositeLogicNodeBuilder` for children since someone may call `apply` on + // the child. + if (!this.children.has(key)) { + this.children.set(key, new CompositeLogicNodeBuilder(this.predicate)); + } + return this.children.get(key)!; + } +} + +/** + * Gets all of the builders that contribute logic to the given child of the parent builder. + */ +function getAllChildren(builder: LogicNodeBuilder, key: PropertyKey): MergableLogicNodeBuilder[] { + if (builder instanceof CompositeLogicNodeBuilder) { + return builder.all.flatMap((node) => getAllChildren(node, key)); + } else if (builder instanceof SimpleLogicNodeBuilder) { + if (builder.children.has(key)) { + return [builder.children.get(key)!]; + } + } else { + throw new Error('Unknown LogicNodeBuilder type'); + } + return []; +} + +/** + * Creates the full `Logic` for a given builder. + */ +function createLogic(builder: LogicNodeBuilder): Logic { + const logic = new Logic(builder.predicate); + if (builder instanceof CompositeLogicNodeBuilder) { + const builtNodes = builder.all.map((b) => b.build()); + for (const node of builtNodes) { + logic.disabled.mergeIn(node.logic.disabled); + logic.hidden.mergeIn(node.logic.hidden); + logic.errors.mergeIn(node.logic.errors); + for (const key of node.logic.getMetadataKeys()) { + logic.getMetadata(key).mergeIn(node.logic.getMetadata(key)); + } + } + } else if (builder instanceof SimpleLogicNodeBuilder) { + logic.disabled.mergeIn(builder.logic.disabled); + logic.hidden.mergeIn(builder.logic.hidden); + logic.errors.mergeIn(builder.logic.errors); + for (const key of builder.logic.getMetadataKeys()) { + logic.getMetadata(key).mergeIn(builder.logic.getMetadata(key)); + } + } else { + throw new Error('Unknown LogicNodeBuilder type'); + } + return logic; +} diff --git a/packages/forms/experimental/test/logic_node_2.spec.ts b/packages/forms/experimental/test/logic_node_2.spec.ts new file mode 100644 index 000000000000..f396e623a510 --- /dev/null +++ b/packages/forms/experimental/test/logic_node_2.spec.ts @@ -0,0 +1,279 @@ +import {signal} from '@angular/core'; +import {FieldContext} from '../public_api'; +import {LogicNodeBuilder} from '../src/logic_node_2'; + +const fakeFieldContext: FieldContext = { + resolve: () => + ({ + $state: {fieldContext: fakeFieldContext}, + }) as any, + value: undefined!, +}; + +describe('LogicNodeBuilder', () => { + it('should build logic', () => { + // (p) => { + // validate(p, () => ({kind: 'root-err'})); + // }; + + const builder = LogicNodeBuilder.newRoot(undefined); + builder.addErrorRule(() => [{kind: 'root-err'}]); + + const logicNode = builder.build(); + expect(logicNode.logic.errors.compute(fakeFieldContext)).toEqual([{kind: 'root-err'}]); + }); + + it('should build child logic', () => { + // (p) => { + // validate(p.a, () => ({kind: 'child-err'})); + // }; + + const builder = LogicNodeBuilder.newRoot(undefined); + builder.getChild('a').addErrorRule(() => [{kind: 'root-err'}]); + + const logicNode = builder.build(); + expect(logicNode.getChild('a').logic.errors.compute(fakeFieldContext)).toEqual([ + {kind: 'root-err'}, + ]); + }); + + it('should build merged logic', () => { + // (p) => { + // validate(p, () => ({kind: 'err-1'})); + // validate(p, () => ({kind: 'err-2'})); + // }; + + const builder = LogicNodeBuilder.newRoot(undefined); + builder.addErrorRule(() => [{kind: 'err-1'}]); + + const builder2 = LogicNodeBuilder.newRoot(undefined); + builder2.addErrorRule(() => [{kind: 'err-2'}]); + builder.mergeIn(builder2); + + const logicNode = builder.build(); + expect(logicNode.logic.errors.compute(fakeFieldContext)).toEqual([ + {kind: 'err-1'}, + {kind: 'err-2'}, + ]); + }); + + it('should build merged child logic', () => { + // (p) => { + // validate(p.a, () => ({kind: 'err-1'})); + // validate(p.a, () => ({kind: 'err-2'})); + // }; + + const builder = LogicNodeBuilder.newRoot(undefined); + builder.getChild('a').addErrorRule(() => [{kind: 'err-1'}]); + + const builder2 = LogicNodeBuilder.newRoot(undefined); + builder2.getChild('a').addErrorRule(() => [{kind: 'err-2'}]); + builder.mergeIn(builder2); + + const logicNode = builder.build(); + expect(logicNode.getChild('a').logic.errors.compute(fakeFieldContext)).toEqual([ + {kind: 'err-1'}, + {kind: 'err-2'}, + ]); + }); + + it('should build logic with predicate', () => { + // applyWhen(p, pred, (p) => { + // validate(p, () => ({kind: 'err-1'})); + // }; + + const pred = signal(true); + const builder = LogicNodeBuilder.newRoot({fn: pred, path: undefined!}); + builder.addErrorRule(() => [{kind: 'err-1'}]); + + const logicNode = builder.build(); + expect(logicNode.logic.errors.compute(fakeFieldContext)).toEqual([{kind: 'err-1'}]); + + pred.set(false); + expect(logicNode.logic.errors.compute(fakeFieldContext)).toEqual([]); + }); + + it('should apply predicate to merged in logic', () => { + // applyWhen(p, pred, (p) => { + // apply(p, (p) => { + // validate(p, () => ({kind: 'err-1'})); + // }); + // }; + + const pred = signal(true); + const builder = LogicNodeBuilder.newRoot({fn: pred, path: undefined!}); + + const builder2 = LogicNodeBuilder.newRoot(undefined); + builder2.addErrorRule(() => [{kind: 'err-1'}]); + builder.mergeIn(builder2); + + const logicNode = builder.build(); + expect(logicNode.logic.errors.compute(fakeFieldContext)).toEqual([{kind: 'err-1'}]); + + pred.set(false); + expect(logicNode.logic.errors.compute(fakeFieldContext)).toEqual([]); + }); + + it('should apply predicate to merged in child logic', () => { + // applyWhen(p, pred, (p) => { + // apply(p, (p) => { + // validate(p.a, () => ({kind: 'err-1'})); + // }); + // }); + + const pred = signal(true); + const builder = LogicNodeBuilder.newRoot({fn: pred, path: undefined!}); + + const builder2 = LogicNodeBuilder.newRoot(undefined); + builder2.getChild('a').addErrorRule(() => [{kind: 'err-1'}]); + builder.mergeIn(builder2); + + const logicNode = builder.build(); + expect(logicNode.getChild('a').logic.errors.compute(fakeFieldContext)).toEqual([ + {kind: 'err-1'}, + ]); + + pred.set(false); + expect(logicNode.getChild('a').logic.errors.compute(fakeFieldContext)).toEqual([]); + }); + + it('should combine predicates', () => { + // applyWhen(p, pred, (p) => { + // applyWhen(p.a, pred2, (a) => { + // validate(a, () => ({kind: 'err-1'})); + // }); + // }); + + const pred = signal(true); + const builder = LogicNodeBuilder.newRoot({fn: pred, path: undefined!}); + + const pred2 = signal(true); + const builder2 = LogicNodeBuilder.newRoot({fn: pred2, path: undefined!}); + builder2.addErrorRule(() => [{kind: 'err-1'}]); + builder.getChild('a').mergeIn(builder2); + + const logicNode = builder.build(); + expect(logicNode.getChild('a').logic.errors.compute(fakeFieldContext)).toEqual([ + {kind: 'err-1'}, + ]); + + pred.set(false); + pred2.set(true); + expect(logicNode.getChild('a').logic.errors.compute(fakeFieldContext)).toEqual([]); + + pred.set(true); + pred2.set(false); + expect(logicNode.getChild('a').logic.errors.compute(fakeFieldContext)).toEqual([]); + }); + + it('should deeply propagate predicates', () => { + // applyWhen(p, pred, (p) => { + // validate(p.a.b, () => ({kind: 'err-1'})); + // applyWhen(p.a, pred2, (a) => { + // validate(a.b, () => ({kind: 'err-2'})); + // applyWhen(a.b, pred3, (b) => { + // validate(b, () => ({kind: 'err-3'})); + // }); + // }); + // }); + + const pred = signal(true); + const builder = LogicNodeBuilder.newRoot({fn: pred, path: undefined!}); + builder + .getChild('a') + .getChild('b') + .addErrorRule(() => [{kind: 'err-1'}]); + + const pred2 = signal(true); + const builder2 = LogicNodeBuilder.newRoot({fn: pred2, path: undefined!}); + builder2.getChild('b').addErrorRule(() => [{kind: 'err-2'}]); + + const pred3 = signal(true); + const builder3 = LogicNodeBuilder.newRoot({fn: pred3, path: undefined!}); + builder3.addErrorRule(() => [{kind: 'err-3'}]); + + builder2.getChild('b').mergeIn(builder3); + builder.getChild('a').mergeIn(builder2); + + const logicNode = builder.build(); + expect(logicNode.getChild('a').getChild('b').logic.errors.compute(fakeFieldContext)).toEqual([ + {kind: 'err-1'}, + {kind: 'err-2'}, + {kind: 'err-3'}, + ]); + + pred.set(true); + pred2.set(true); + pred3.set(false); + expect(logicNode.getChild('a').getChild('b').logic.errors.compute(fakeFieldContext)).toEqual([ + {kind: 'err-1'}, + {kind: 'err-2'}, + ]); + + pred.set(true); + pred2.set(false); + pred3.set(true); + expect(logicNode.getChild('a').getChild('b').logic.errors.compute(fakeFieldContext)).toEqual([ + {kind: 'err-1'}, + ]); + + pred.set(false); + pred2.set(true); + pred3.set(true); + expect(logicNode.getChild('a').getChild('b').logic.errors.compute(fakeFieldContext)).toEqual( + [], + ); + }); + + it('should preserve ordering across merges', () => { + // (p) => { + // validate(p, () => ({kind: 'err-1'})); + // apply(p, (p) => { + // validate(p, () => ({kind: 'err-2'})); + // }) + // validate(p, () => ({kind: 'err-3'})); + // }; + + const builder = LogicNodeBuilder.newRoot(undefined); + builder.addErrorRule(() => [{kind: 'err-1'}]); + + const builder2 = LogicNodeBuilder.newRoot(undefined); + builder2.addErrorRule(() => [{kind: 'err-2'}]); + builder.mergeIn(builder2); + + builder.addErrorRule(() => [{kind: 'err-3'}]); + + const logicNode = builder.build(); + expect(logicNode.logic.errors.compute(fakeFieldContext)).toEqual([ + {kind: 'err-1'}, + {kind: 'err-2'}, + {kind: 'err-3'}, + ]); + }); + + it('should preserve child ordering across merges', () => { + // (p) => { + // validate(p.a, () => ({kind: 'err-1'})); + // apply(p, (p) => { + // validate(p.a, () => ({kind: 'err-2'})); + // }) + // validate(p.a, () => ({kind: 'err-3'})); + // }; + + const builder = LogicNodeBuilder.newRoot(undefined); + builder.getChild('a').addErrorRule(() => [{kind: 'err-1'}]); + + const builder2 = LogicNodeBuilder.newRoot(undefined); + builder2.getChild('a').addErrorRule(() => [{kind: 'err-2'}]); + builder.mergeIn(builder2); + + builder.getChild('a').addErrorRule(() => [{kind: 'err-3'}]); + + const logicNode = builder.build(); + expect(logicNode.getChild('a').logic.errors.compute(fakeFieldContext)).toEqual([ + {kind: 'err-1'}, + {kind: 'err-2'}, + {kind: 'err-3'}, + ]); + }); +}); From de4b134e8ad41225024d7931f4c26fbb6028212b Mon Sep 17 00:00:00 2001 From: Miles Malerba Date: Fri, 2 May 2025 05:38:55 +0000 Subject: [PATCH 2/4] refactor: maintain predicates as list --- packages/forms/experimental/src/logic_node.ts | 42 ++-- .../forms/experimental/src/logic_node_2.ts | 70 +++++-- .../experimental/test/logic_node_2.spec.ts | 179 ++++++++++++------ 3 files changed, 195 insertions(+), 96 deletions(-) diff --git a/packages/forms/experimental/src/logic_node.ts b/packages/forms/experimental/src/logic_node.ts index a4181d0cd2cb..c2ed29588804 100644 --- a/packages/forms/experimental/src/logic_node.ts +++ b/packages/forms/experimental/src/logic_node.ts @@ -33,11 +33,13 @@ export class FieldLogicNode { private readonly metadata = new Map, AbstractLogic>(); private readonly children = new Map(); + private readonly predicates: Predicate[]; - private constructor(private predicate: Predicate | undefined) { - this.hidden = new BooleanOrLogic(predicate); - this.disabled = new BooleanOrLogic(predicate); - this.errors = new ArrayMergeLogic(predicate); + private constructor(predicate: Predicate | undefined) { + this.predicates = predicate !== undefined ? [predicate] : []; + this.hidden = new BooleanOrLogic(this.predicates); + this.disabled = new BooleanOrLogic(this.predicates); + this.errors = new ArrayMergeLogic(this.predicates); } get element(): FieldLogicNode { @@ -46,7 +48,7 @@ export class FieldLogicNode { getMetadata(key: MetadataKey): AbstractLogic { if (!this.metadata.has(key as MetadataKey)) { - this.metadata.set(key as MetadataKey, new MetadataMergeLogic(this.predicate, key)); + this.metadata.set(key as MetadataKey, new MetadataMergeLogic(this.predicates, key)); } return this.metadata.get(key as MetadataKey)! as AbstractLogic; } @@ -64,7 +66,7 @@ export class FieldLogicNode { */ getChild(key: PropertyKey): FieldLogicNode { if (!this.children.has(key)) { - this.children.set(key, new FieldLogicNode(this.predicate)); + this.children.set(key, new FieldLogicNode(this.predicates[0])); } return this.children.get(key)!; } @@ -95,19 +97,19 @@ export class FieldLogicNode { export abstract class AbstractLogic { protected readonly fns: Array> = []; - constructor(private predicate: Predicate | undefined) {} + constructor(private predicates: ReadonlyArray) {} abstract compute(arg: FieldContext): TReturn; abstract get defaultValue(): TValue; push(logicFn: LogicFn) { - this.fns.push(wrapWithPredicate(this.predicate, logicFn, this.defaultValue)); + this.fns.push(wrapWithPredicates(this.predicates, logicFn, this.defaultValue)); } mergeIn(other: AbstractLogic) { - const fns = this.predicate - ? other.fns.map((fn) => wrapWithPredicate(this.predicate, fn, this.defaultValue)) + const fns = this.predicates + ? other.fns.map((fn) => wrapWithPredicates(this.predicates, fn, this.defaultValue)) : other.fns; this.fns.push(...fns); } @@ -152,10 +154,10 @@ export class MetadataMergeLogic extends AbstractLogic { } constructor( - predicate: Predicate | undefined, + predicates: ReadonlyArray, private key: MetadataKey, ) { - super(predicate); + super(predicates); } override compute(arg: FieldContext): T { @@ -163,19 +165,21 @@ export class MetadataMergeLogic extends AbstractLogic { } } -function wrapWithPredicate( - predicate: Predicate | undefined, +function wrapWithPredicates( + predicates: ReadonlyArray, logicFn: LogicFn, defaultValue: TReturn, ) { - if (predicate === undefined) { + if (predicates.length === 0) { return logicFn; } return (arg: FieldContext): TReturn => { - const predicateField = arg.resolve(predicate.path).$state as FieldNode; - if (!predicate.fn(predicateField.fieldContext)) { - // don't actually run the user function - return defaultValue; + for (const predicate of predicates) { + const predicateField = arg.resolve(predicate.path).$state as FieldNode; + if (!predicate.fn(predicateField.fieldContext)) { + // don't actually run the user function + return defaultValue; + } } return logicFn(arg); }; diff --git a/packages/forms/experimental/src/logic_node_2.ts b/packages/forms/experimental/src/logic_node_2.ts index d8f487d16c59..147d9c629027 100644 --- a/packages/forms/experimental/src/logic_node_2.ts +++ b/packages/forms/experimental/src/logic_node_2.ts @@ -17,15 +17,15 @@ export class Logic { readonly errors: ArrayMergeLogic; private readonly metadata = new Map, AbstractLogic>(); - constructor(private predicate: Predicate | undefined) { - this.hidden = new BooleanOrLogic(predicate); - this.disabled = new BooleanOrLogic(predicate); - this.errors = new ArrayMergeLogic(predicate); + constructor(private predicates: ReadonlyArray) { + this.hidden = new BooleanOrLogic(predicates); + this.disabled = new BooleanOrLogic(predicates); + this.errors = new ArrayMergeLogic(predicates); } getMetadata(key: MetadataKey): AbstractLogic { if (!this.metadata.has(key as MetadataKey)) { - this.metadata.set(key as MetadataKey, new MetadataMergeLogic(this.predicate, key)); + this.metadata.set(key as MetadataKey, new MetadataMergeLogic(this.predicates, key)); } return this.metadata.get(key as MetadataKey)! as AbstractLogic; } @@ -50,19 +50,19 @@ export class LogicNode { readonly logic: Logic; constructor(private builder: LogicNodeBuilder | undefined) { - this.logic = builder ? createLogic(builder) : new Logic(undefined); + this.logic = builder ? createLogic(builder) : new Logic([]); } getChild(key: PropertyKey): LogicNode { // The logic for a particular child may be spread across multiple builders. We lazily combine // this logic at the time the child logic node is requested to be created. const childBuilders = this.builder ? getAllChildren(this.builder, key) : []; - if (childBuilders.length === 0) { - return new LogicNode(undefined); + if (childBuilders.length <= 1) { + return new LogicNode(childBuilders[0]); } else { - // If there are child builders, merge them under a new builder to ensure the predicate is - // propagated. - const combined = LogicNodeBuilder.newRoot(this.builder?.predicate); + // If there are multiple child builders, combine them all into one we ca pass to the new logic + // node. + const combined = LogicNodeBuilder.newRoot(); for (const child of childBuilders) { combined.mergeIn(child); } @@ -75,20 +75,21 @@ export class LogicNode { * A builder for `LogicNode`, which itself is a tree. */ export abstract class LogicNodeBuilder { - protected constructor(readonly predicate: Predicate | undefined) {} + protected constructor(readonly predicates: ReadonlyArray) {} abstract addHiddenRule(logic: LogicFn): void; abstract addDisabledRule(logic: LogicFn): void; abstract addErrorRule(logic: LogicFn): void; abstract addMetadataRule(key: MetadataKey, logic: LogicFn): void; abstract getChild(key: PropertyKey): MergableLogicNodeBuilder; + abstract predicate(predicates: ReadonlyArray): LogicNodeBuilder; build(): LogicNode { return new LogicNode(this); } - static newRoot(predicate: Predicate | undefined): MergableLogicNodeBuilder { - return new CompositeLogicNodeBuilder(predicate); + static newRoot(): MergableLogicNodeBuilder { + return new CompositeLogicNodeBuilder([]); } } @@ -96,7 +97,8 @@ export abstract class LogicNodeBuilder { * A subclass of `LogicNodeBuilder` that supports merging with other logic trees. */ export interface MergableLogicNodeBuilder extends LogicNodeBuilder { - mergeIn(other: MergableLogicNodeBuilder): void; + mergeIn(other: MergableLogicNodeBuilder, predicate?: Predicate): void; + predicate(predicates: ReadonlyArray): MergableLogicNodeBuilder; } /** @@ -127,12 +129,28 @@ class CompositeLogicNodeBuilder extends LogicNodeBuilder implements MergableLogi return this.getCurrent().getChild(key); } - mergeIn(other: MergableLogicNodeBuilder): void { + override predicate(predicates: ReadonlyArray) { + const newPredicates = [...this.predicates, ...predicates]; + const clone = new CompositeLogicNodeBuilder(newPredicates); + clone.all.push(...this.all.map((b) => b.predicate(newPredicates))); + clone.current = this.current?.predicate(newPredicates); + return clone; + } + + mergeIn(other: MergableLogicNodeBuilder, predicate?: Predicate): void { // Add the other builder to our collection, we'll defer the actual merging of the logic until // the logic node is requested to be created. In order to preserve the original ordering of the // rules, we close off the current builder to any further edits. If additional logic is added, // a new current builder will be created to capture it. - this.all.push(other); + const predicates = [...this.predicates]; + if (predicate) { + predicates.push(predicate); + } + if (predicates.length !== 0) { + this.all.push(other.predicate(predicates)); + } else { + this.all.push(other); + } this.current = undefined; } @@ -141,7 +159,7 @@ class CompositeLogicNodeBuilder extends LogicNodeBuilder implements MergableLogi // builder, a new one is created. In order to preserve the original ordering of the rules, we // clear the current builder whenever a separate builder tree is merged in. if (this.current === undefined) { - this.current = new SimpleLogicNodeBuilder(this.predicate); + this.current = new SimpleLogicNodeBuilder(this.predicates); this.all.push(this.current); } return this.current; @@ -156,7 +174,7 @@ class CompositeLogicNodeBuilder extends LogicNodeBuilder implements MergableLogi * to the user it should always be a `CompositeLogicNodeBuilder`. */ class SimpleLogicNodeBuilder extends LogicNodeBuilder { - readonly logic = new Logic(undefined); + logic = new Logic([]); readonly children = new Map(); override addHiddenRule(logic: LogicFn): void { @@ -175,11 +193,21 @@ class SimpleLogicNodeBuilder extends LogicNodeBuilder { this.logic.getMetadata(key).push(logic); } + override predicate(predicates: ReadonlyArray) { + const newPredicates = [...this.predicates, ...predicates]; + const clone = new SimpleLogicNodeBuilder(newPredicates); + for (const [prop, child] of this.children) { + clone.children.set(prop, child.predicate(newPredicates)); + } + clone.logic = this.logic; + return clone; + } + override getChild(key: PropertyKey): MergableLogicNodeBuilder { // We always create a `CompositeLogicNodeBuilder` for children since someone may call `apply` on // the child. if (!this.children.has(key)) { - this.children.set(key, new CompositeLogicNodeBuilder(this.predicate)); + this.children.set(key, new CompositeLogicNodeBuilder(this.predicates)); } return this.children.get(key)!; } @@ -205,7 +233,7 @@ function getAllChildren(builder: LogicNodeBuilder, key: PropertyKey): MergableLo * Creates the full `Logic` for a given builder. */ function createLogic(builder: LogicNodeBuilder): Logic { - const logic = new Logic(builder.predicate); + const logic = new Logic(builder.predicates); if (builder instanceof CompositeLogicNodeBuilder) { const builtNodes = builder.all.map((b) => b.build()); for (const node of builtNodes) { diff --git a/packages/forms/experimental/test/logic_node_2.spec.ts b/packages/forms/experimental/test/logic_node_2.spec.ts index f396e623a510..56654fd2f7e3 100644 --- a/packages/forms/experimental/test/logic_node_2.spec.ts +++ b/packages/forms/experimental/test/logic_node_2.spec.ts @@ -1,5 +1,6 @@ import {signal} from '@angular/core'; import {FieldContext} from '../public_api'; +import {DYNAMIC} from '../src/logic_node'; import {LogicNodeBuilder} from '../src/logic_node_2'; const fakeFieldContext: FieldContext = { @@ -16,7 +17,7 @@ describe('LogicNodeBuilder', () => { // validate(p, () => ({kind: 'root-err'})); // }; - const builder = LogicNodeBuilder.newRoot(undefined); + const builder = LogicNodeBuilder.newRoot(); builder.addErrorRule(() => [{kind: 'root-err'}]); const logicNode = builder.build(); @@ -28,7 +29,7 @@ describe('LogicNodeBuilder', () => { // validate(p.a, () => ({kind: 'child-err'})); // }; - const builder = LogicNodeBuilder.newRoot(undefined); + const builder = LogicNodeBuilder.newRoot(); builder.getChild('a').addErrorRule(() => [{kind: 'root-err'}]); const logicNode = builder.build(); @@ -43,10 +44,10 @@ describe('LogicNodeBuilder', () => { // validate(p, () => ({kind: 'err-2'})); // }; - const builder = LogicNodeBuilder.newRoot(undefined); + const builder = LogicNodeBuilder.newRoot(); builder.addErrorRule(() => [{kind: 'err-1'}]); - const builder2 = LogicNodeBuilder.newRoot(undefined); + const builder2 = LogicNodeBuilder.newRoot(); builder2.addErrorRule(() => [{kind: 'err-2'}]); builder.mergeIn(builder2); @@ -63,10 +64,10 @@ describe('LogicNodeBuilder', () => { // validate(p.a, () => ({kind: 'err-2'})); // }; - const builder = LogicNodeBuilder.newRoot(undefined); + const builder = LogicNodeBuilder.newRoot(); builder.getChild('a').addErrorRule(() => [{kind: 'err-1'}]); - const builder2 = LogicNodeBuilder.newRoot(undefined); + const builder2 = LogicNodeBuilder.newRoot(); builder2.getChild('a').addErrorRule(() => [{kind: 'err-2'}]); builder.mergeIn(builder2); @@ -78,13 +79,18 @@ describe('LogicNodeBuilder', () => { }); it('should build logic with predicate', () => { - // applyWhen(p, pred, (p) => { - // validate(p, () => ({kind: 'err-1'})); - // }; + // (p) => { + // applyWhen(p, pred, (p) => { + // validate(p, () => ({kind: 'err-1'})); + // }); + // } + + const builder = LogicNodeBuilder.newRoot(); const pred = signal(true); - const builder = LogicNodeBuilder.newRoot({fn: pred, path: undefined!}); - builder.addErrorRule(() => [{kind: 'err-1'}]); + const builder2 = LogicNodeBuilder.newRoot(); + builder2.addErrorRule(() => [{kind: 'err-1'}]); + builder.mergeIn(builder2, {fn: pred, path: undefined!}); const logicNode = builder.build(); expect(logicNode.logic.errors.compute(fakeFieldContext)).toEqual([{kind: 'err-1'}]); @@ -94,18 +100,24 @@ describe('LogicNodeBuilder', () => { }); it('should apply predicate to merged in logic', () => { - // applyWhen(p, pred, (p) => { - // apply(p, (p) => { - // validate(p, () => ({kind: 'err-1'})); + // (p) => { + // applyWhen(p, pred, (p) => { + // apply(p, (p) => { + // validate(p, () => ({kind: 'err-1'})); + // }); // }); - // }; + // } + + const builder = LogicNodeBuilder.newRoot(); const pred = signal(true); - const builder = LogicNodeBuilder.newRoot({fn: pred, path: undefined!}); + const builder2 = LogicNodeBuilder.newRoot(); - const builder2 = LogicNodeBuilder.newRoot(undefined); - builder2.addErrorRule(() => [{kind: 'err-1'}]); - builder.mergeIn(builder2); + const builder3 = LogicNodeBuilder.newRoot(); + builder3.addErrorRule(() => [{kind: 'err-1'}]); + + builder2.mergeIn(builder3); + builder.mergeIn(builder2, {fn: pred, path: undefined!}); const logicNode = builder.build(); expect(logicNode.logic.errors.compute(fakeFieldContext)).toEqual([{kind: 'err-1'}]); @@ -115,18 +127,24 @@ describe('LogicNodeBuilder', () => { }); it('should apply predicate to merged in child logic', () => { - // applyWhen(p, pred, (p) => { - // apply(p, (p) => { - // validate(p.a, () => ({kind: 'err-1'})); + // (p) => { + // applyWhen(p, pred, (p) => { + // apply(p, (p) => { + // validate(p.a, () => ({kind: 'err-1'})); + // }); // }); - // }); + // } + + const builder = LogicNodeBuilder.newRoot(); const pred = signal(true); - const builder = LogicNodeBuilder.newRoot({fn: pred, path: undefined!}); + const builder2 = LogicNodeBuilder.newRoot(); - const builder2 = LogicNodeBuilder.newRoot(undefined); - builder2.getChild('a').addErrorRule(() => [{kind: 'err-1'}]); - builder.mergeIn(builder2); + const builder3 = LogicNodeBuilder.newRoot(); + builder3.getChild('a').addErrorRule(() => [{kind: 'err-1'}]); + + builder2.mergeIn(builder3); + builder.mergeIn(builder2, {fn: pred, path: undefined!}); const logicNode = builder.build(); expect(logicNode.getChild('a').logic.errors.compute(fakeFieldContext)).toEqual([ @@ -138,19 +156,25 @@ describe('LogicNodeBuilder', () => { }); it('should combine predicates', () => { - // applyWhen(p, pred, (p) => { - // applyWhen(p.a, pred2, (a) => { - // validate(a, () => ({kind: 'err-1'})); + // (p) => { + // applyWhen(p, pred, (p) => { + // applyWhen(p.a, pred2, (a) => { + // validate(a, () => ({kind: 'err-1'})); + // }); // }); - // }); + // } + + const builder = LogicNodeBuilder.newRoot(); const pred = signal(true); - const builder = LogicNodeBuilder.newRoot({fn: pred, path: undefined!}); + const builder2 = LogicNodeBuilder.newRoot(); const pred2 = signal(true); - const builder2 = LogicNodeBuilder.newRoot({fn: pred2, path: undefined!}); - builder2.addErrorRule(() => [{kind: 'err-1'}]); - builder.getChild('a').mergeIn(builder2); + const builder3 = LogicNodeBuilder.newRoot(); + builder3.addErrorRule(() => [{kind: 'err-1'}]); + + builder2.getChild('a').mergeIn(builder3, {fn: pred2, path: undefined!}); + builder.mergeIn(builder2, {fn: pred, path: undefined!}); const logicNode = builder.build(); expect(logicNode.getChild('a').logic.errors.compute(fakeFieldContext)).toEqual([ @@ -166,34 +190,38 @@ describe('LogicNodeBuilder', () => { expect(logicNode.getChild('a').logic.errors.compute(fakeFieldContext)).toEqual([]); }); - it('should deeply propagate predicates', () => { - // applyWhen(p, pred, (p) => { - // validate(p.a.b, () => ({kind: 'err-1'})); - // applyWhen(p.a, pred2, (a) => { - // validate(a.b, () => ({kind: 'err-2'})); - // applyWhen(a.b, pred3, (b) => { - // validate(b, () => ({kind: 'err-3'})); + it('should propagate predicates through deep application', () => { + // (p) => { + // applyWhen(p, pred, (p) => { + // validate(p.a.b, () => ({kind: 'err-1'})); + // applyWhen(p.a, pred2, (a) => { + // validate(a.b, () => ({kind: 'err-2'})); + // applyWhen(a.b, pred3, (b) => { + // validate(b, () => ({kind: 'err-3'})); + // }); // }); // }); - // }); + // } + + const builder = LogicNodeBuilder.newRoot(); const pred = signal(true); - const builder = LogicNodeBuilder.newRoot({fn: pred, path: undefined!}); - builder + const builder2 = LogicNodeBuilder.newRoot(); + builder2 .getChild('a') .getChild('b') .addErrorRule(() => [{kind: 'err-1'}]); const pred2 = signal(true); - const builder2 = LogicNodeBuilder.newRoot({fn: pred2, path: undefined!}); - builder2.getChild('b').addErrorRule(() => [{kind: 'err-2'}]); + const builder3 = LogicNodeBuilder.newRoot(); + builder3.getChild('b').addErrorRule(() => [{kind: 'err-2'}]); const pred3 = signal(true); - const builder3 = LogicNodeBuilder.newRoot({fn: pred3, path: undefined!}); - builder3.addErrorRule(() => [{kind: 'err-3'}]); - - builder2.getChild('b').mergeIn(builder3); - builder.getChild('a').mergeIn(builder2); + const builder4 = LogicNodeBuilder.newRoot(); + builder4.addErrorRule(() => [{kind: 'err-3'}]); + builder3.getChild('b').mergeIn(builder4, {fn: pred3, path: undefined!}); + builder2.getChild('a').mergeIn(builder3, {fn: pred2, path: undefined!}); + builder.mergeIn(builder2, {fn: pred, path: undefined!}); const logicNode = builder.build(); expect(logicNode.getChild('a').getChild('b').logic.errors.compute(fakeFieldContext)).toEqual([ @@ -225,6 +253,45 @@ describe('LogicNodeBuilder', () => { ); }); + it('should propagate predicates through deep child access', () => { + // (p) => { + // applyWhen(p, pred, (p) => { + // applyEach(p.items, (i) => { + // validate(i.last, () => ({kind: 'err-1'})); + // }); + // }); + // }; + + const builder = LogicNodeBuilder.newRoot(); + + const pred = signal(true); + const builder2 = LogicNodeBuilder.newRoot(); + + const builder3 = LogicNodeBuilder.newRoot(); + builder3.getChild('last').addErrorRule(() => [{kind: 'err-1'}]); + + builder2.getChild('items').getChild(DYNAMIC).mergeIn(builder3); + builder.mergeIn(builder2, {fn: pred, path: undefined!}); + + const logicNode = builder.build(); + expect( + logicNode + .getChild('items') + .getChild(DYNAMIC) + .getChild('last') + .logic.errors.compute(fakeFieldContext), + ).toEqual([{kind: 'err-1'}]); + + pred.set(false); + expect( + logicNode + .getChild('items') + .getChild(DYNAMIC) + .getChild('last') + .logic.errors.compute(fakeFieldContext), + ).toEqual([]); + }); + it('should preserve ordering across merges', () => { // (p) => { // validate(p, () => ({kind: 'err-1'})); @@ -234,10 +301,10 @@ describe('LogicNodeBuilder', () => { // validate(p, () => ({kind: 'err-3'})); // }; - const builder = LogicNodeBuilder.newRoot(undefined); + const builder = LogicNodeBuilder.newRoot(); builder.addErrorRule(() => [{kind: 'err-1'}]); - const builder2 = LogicNodeBuilder.newRoot(undefined); + const builder2 = LogicNodeBuilder.newRoot(); builder2.addErrorRule(() => [{kind: 'err-2'}]); builder.mergeIn(builder2); @@ -260,10 +327,10 @@ describe('LogicNodeBuilder', () => { // validate(p.a, () => ({kind: 'err-3'})); // }; - const builder = LogicNodeBuilder.newRoot(undefined); + const builder = LogicNodeBuilder.newRoot(); builder.getChild('a').addErrorRule(() => [{kind: 'err-1'}]); - const builder2 = LogicNodeBuilder.newRoot(undefined); + const builder2 = LogicNodeBuilder.newRoot(); builder2.getChild('a').addErrorRule(() => [{kind: 'err-2'}]); builder.mergeIn(builder2); From 86f8370532397ddaa799c0233629c314fd6d80eb Mon Sep 17 00:00:00 2001 From: Miles Malerba Date: Tue, 6 May 2025 23:00:15 +0000 Subject: [PATCH 3/4] refactor: refactor builder api --- .../forms/experimental/src/logic_node_2.ts | 225 +++++++++--------- 1 file changed, 108 insertions(+), 117 deletions(-) diff --git a/packages/forms/experimental/src/logic_node_2.ts b/packages/forms/experimental/src/logic_node_2.ts index 147d9c629027..2fc651e0fc45 100644 --- a/packages/forms/experimental/src/logic_node_2.ts +++ b/packages/forms/experimental/src/logic_node_2.ts @@ -7,137 +7,61 @@ import { Predicate, } from './logic_node'; -/** - * Container for all the different types of logic that can be applied to a field - * (disabled, hidden, errors, etc.) - */ -export class Logic { - readonly hidden: BooleanOrLogic; - readonly disabled: BooleanOrLogic; - readonly errors: ArrayMergeLogic; - private readonly metadata = new Map, AbstractLogic>(); - - constructor(private predicates: ReadonlyArray) { - this.hidden = new BooleanOrLogic(predicates); - this.disabled = new BooleanOrLogic(predicates); - this.errors = new ArrayMergeLogic(predicates); - } - - getMetadata(key: MetadataKey): AbstractLogic { - if (!this.metadata.has(key as MetadataKey)) { - this.metadata.set(key as MetadataKey, new MetadataMergeLogic(this.predicates, key)); - } - return this.metadata.get(key as MetadataKey)! as AbstractLogic; - } - - readMetadata(key: MetadataKey, arg: FieldContext): T { - if (this.metadata.has(key as MetadataKey)) { - return this.metadata.get(key as MetadataKey)!.compute(arg) as T; - } else { - return key.defaultValue; - } - } - - getMetadataKeys() { - return this.metadata.keys(); - } -} - -/** - * A tree structure of `Logic` corresponding to a tree of fields. - */ -export class LogicNode { - readonly logic: Logic; - - constructor(private builder: LogicNodeBuilder | undefined) { - this.logic = builder ? createLogic(builder) : new Logic([]); - } - - getChild(key: PropertyKey): LogicNode { - // The logic for a particular child may be spread across multiple builders. We lazily combine - // this logic at the time the child logic node is requested to be created. - const childBuilders = this.builder ? getAllChildren(this.builder, key) : []; - if (childBuilders.length <= 1) { - return new LogicNode(childBuilders[0]); - } else { - // If there are multiple child builders, combine them all into one we ca pass to the new logic - // node. - const combined = LogicNodeBuilder.newRoot(); - for (const child of childBuilders) { - combined.mergeIn(child); - } - return new LogicNode(combined); - } - } -} - -/** - * A builder for `LogicNode`, which itself is a tree. - */ -export abstract class LogicNodeBuilder { - protected constructor(readonly predicates: ReadonlyArray) {} +abstract class AbstractLogicNodeBuilder { + constructor(readonly predicates: ReadonlyArray) {} abstract addHiddenRule(logic: LogicFn): void; abstract addDisabledRule(logic: LogicFn): void; abstract addErrorRule(logic: LogicFn): void; abstract addMetadataRule(key: MetadataKey, logic: LogicFn): void; - abstract getChild(key: PropertyKey): MergableLogicNodeBuilder; - abstract predicate(predicates: ReadonlyArray): LogicNodeBuilder; + abstract getChild(key: PropertyKey): LogicNodeBuilder; + abstract predicate(predicates: ReadonlyArray): AbstractLogicNodeBuilder; build(): LogicNode { return new LogicNode(this); } - - static newRoot(): MergableLogicNodeBuilder { - return new CompositeLogicNodeBuilder([]); - } } /** - * A subclass of `LogicNodeBuilder` that supports merging with other logic trees. + * A builder for `LogicNode`. Used to add logic to the final `LogicNode` tree. */ -export interface MergableLogicNodeBuilder extends LogicNodeBuilder { - mergeIn(other: MergableLogicNodeBuilder, predicate?: Predicate): void; - predicate(predicates: ReadonlyArray): MergableLogicNodeBuilder; -} +export class LogicNodeBuilder extends AbstractLogicNodeBuilder { + private current: NonMergableLogicNodeBuilder | undefined; + readonly all: AbstractLogicNodeBuilder[] = []; -/** - * A `MergableLogicNodeBuilder` that delegates to its "current" builder and creates a new current - * builder after another builder tree is merged in. - */ -class CompositeLogicNodeBuilder extends LogicNodeBuilder implements MergableLogicNodeBuilder { - private current: SimpleLogicNodeBuilder | undefined; - readonly all: LogicNodeBuilder[] = []; + constructor(predicates: ReadonlyArray) { + super(predicates); + } - override addHiddenRule(logic: LogicFn): void { + addHiddenRule(logic: LogicFn): void { this.getCurrent().addHiddenRule(logic); } - override addDisabledRule(logic: LogicFn): void { + addDisabledRule(logic: LogicFn): void { this.getCurrent().addDisabledRule(logic); } - override addErrorRule(logic: LogicFn): void { + addErrorRule(logic: LogicFn): void { this.getCurrent().addErrorRule(logic); } - override addMetadataRule(key: MetadataKey, logic: LogicFn): void { + addMetadataRule(key: MetadataKey, logic: LogicFn): void { this.getCurrent().addMetadataRule(key, logic); } - override getChild(key: PropertyKey): MergableLogicNodeBuilder { + getChild(key: PropertyKey): LogicNodeBuilder { return this.getCurrent().getChild(key); } - override predicate(predicates: ReadonlyArray) { + predicate(predicates: ReadonlyArray) { const newPredicates = [...this.predicates, ...predicates]; - const clone = new CompositeLogicNodeBuilder(newPredicates); + const clone = new LogicNodeBuilder(newPredicates); clone.all.push(...this.all.map((b) => b.predicate(newPredicates))); clone.current = this.current?.predicate(newPredicates); return clone; } - mergeIn(other: MergableLogicNodeBuilder, predicate?: Predicate): void { + mergeIn(other: LogicNodeBuilder, predicate?: Predicate): void { // Add the other builder to our collection, we'll defer the actual merging of the logic until // the logic node is requested to be created. In order to preserve the original ordering of the // rules, we close off the current builder to any further edits. If additional logic is added, @@ -154,28 +78,29 @@ class CompositeLogicNodeBuilder extends LogicNodeBuilder implements MergableLogi this.current = undefined; } - private getCurrent(): SimpleLogicNodeBuilder { + private getCurrent(): NonMergableLogicNodeBuilder { // All rules added to this builder get added on to the current builder. If there is no current // builder, a new one is created. In order to preserve the original ordering of the rules, we // clear the current builder whenever a separate builder tree is merged in. if (this.current === undefined) { - this.current = new SimpleLogicNodeBuilder(this.predicates); + this.current = new NonMergableLogicNodeBuilder(this.predicates); this.all.push(this.current); } return this.current; } + + static newRoot(): LogicNodeBuilder { + return new LogicNodeBuilder([]); + } } /** - * A `LogicNodeBuilder` that directly adds logic to its backing `Logic` instance. - * - * The user should not be given a reference to this class, it is used internally to keep track of - * concrete tree chunks within the `CompositeLogicNodeBuilder`. When handing a `LogicNodeBuilder` - * to the user it should always be a `CompositeLogicNodeBuilder`. + * A type of `AbstractLogicNodeBuilder` used internally by the `LogicNodeBuilder` to record "pure" + * chunks of logic that do not require merging in other builders. */ -class SimpleLogicNodeBuilder extends LogicNodeBuilder { +class NonMergableLogicNodeBuilder extends AbstractLogicNodeBuilder { logic = new Logic([]); - readonly children = new Map(); + readonly children = new Map(); override addHiddenRule(logic: LogicFn): void { this.logic.hidden.push(logic); @@ -195,7 +120,7 @@ class SimpleLogicNodeBuilder extends LogicNodeBuilder { override predicate(predicates: ReadonlyArray) { const newPredicates = [...this.predicates, ...predicates]; - const clone = new SimpleLogicNodeBuilder(newPredicates); + const clone = new NonMergableLogicNodeBuilder(newPredicates); for (const [prop, child] of this.children) { clone.children.set(prop, child.predicate(newPredicates)); } @@ -203,23 +128,86 @@ class SimpleLogicNodeBuilder extends LogicNodeBuilder { return clone; } - override getChild(key: PropertyKey): MergableLogicNodeBuilder { - // We always create a `CompositeLogicNodeBuilder` for children since someone may call `apply` on - // the child. + override getChild(key: PropertyKey): LogicNodeBuilder { if (!this.children.has(key)) { - this.children.set(key, new CompositeLogicNodeBuilder(this.predicates)); + this.children.set(key, new LogicNodeBuilder(this.predicates)); } return this.children.get(key)!; } } +/** + * Container for all the different types of logic that can be applied to a field + * (disabled, hidden, errors, etc.) + */ +export class Logic { + readonly hidden: BooleanOrLogic; + readonly disabled: BooleanOrLogic; + readonly errors: ArrayMergeLogic; + private readonly metadata = new Map, AbstractLogic>(); + + constructor(private predicates: ReadonlyArray) { + this.hidden = new BooleanOrLogic(predicates); + this.disabled = new BooleanOrLogic(predicates); + this.errors = new ArrayMergeLogic(predicates); + } + + getMetadata(key: MetadataKey): AbstractLogic { + if (!this.metadata.has(key as MetadataKey)) { + this.metadata.set(key as MetadataKey, new MetadataMergeLogic(this.predicates, key)); + } + return this.metadata.get(key as MetadataKey)! as AbstractLogic; + } + + readMetadata(key: MetadataKey, arg: FieldContext): T { + if (this.metadata.has(key as MetadataKey)) { + return this.metadata.get(key as MetadataKey)!.compute(arg) as T; + } else { + return key.defaultValue; + } + } + + getMetadataKeys() { + return this.metadata.keys(); + } +} + +/** + * A tree structure of `Logic` corresponding to a tree of fields. + */ +export class LogicNode { + readonly logic: Logic; + + constructor(private builder: AbstractLogicNodeBuilder | undefined) { + this.logic = builder ? createLogic(builder) : new Logic([]); + } + + // TODO: cache here, or just rely on the user of this API to do caching? + getChild(key: PropertyKey): LogicNode { + // The logic for a particular child may be spread across multiple builders. We lazily combine + // this logic at the time the child logic node is requested to be created. + const childBuilders = this.builder ? getAllChildren(this.builder, key) : []; + if (childBuilders.length <= 1) { + return new LogicNode(childBuilders[0]); + } else { + // If there are multiple child builders, combine them all into one we ca pass to the new logic + // node. + const combined = LogicNodeBuilder.newRoot(); + for (const child of childBuilders) { + combined.mergeIn(child); + } + return new LogicNode(combined); + } + } +} + /** * Gets all of the builders that contribute logic to the given child of the parent builder. */ -function getAllChildren(builder: LogicNodeBuilder, key: PropertyKey): MergableLogicNodeBuilder[] { - if (builder instanceof CompositeLogicNodeBuilder) { +function getAllChildren(builder: AbstractLogicNodeBuilder, key: PropertyKey): LogicNodeBuilder[] { + if (builder instanceof LogicNodeBuilder) { return builder.all.flatMap((node) => getAllChildren(node, key)); - } else if (builder instanceof SimpleLogicNodeBuilder) { + } else if (builder instanceof NonMergableLogicNodeBuilder) { if (builder.children.has(key)) { return [builder.children.get(key)!]; } @@ -232,9 +220,10 @@ function getAllChildren(builder: LogicNodeBuilder, key: PropertyKey): MergableLo /** * Creates the full `Logic` for a given builder. */ -function createLogic(builder: LogicNodeBuilder): Logic { - const logic = new Logic(builder.predicates); - if (builder instanceof CompositeLogicNodeBuilder) { +function createLogic(builder: AbstractLogicNodeBuilder): Logic { + if (builder instanceof LogicNodeBuilder) { + // Build the logic for all sub-builders, and merge them into one. + const logic = new Logic([]); const builtNodes = builder.all.map((b) => b.build()); for (const node of builtNodes) { logic.disabled.mergeIn(node.logic.disabled); @@ -244,15 +233,17 @@ function createLogic(builder: LogicNodeBuilder): Logic { logic.getMetadata(key).mergeIn(node.logic.getMetadata(key)); } } - } else if (builder instanceof SimpleLogicNodeBuilder) { + return logic; + } else if (builder instanceof NonMergableLogicNodeBuilder) { + const logic = new Logic(builder.predicates); logic.disabled.mergeIn(builder.logic.disabled); logic.hidden.mergeIn(builder.logic.hidden); logic.errors.mergeIn(builder.logic.errors); for (const key of builder.logic.getMetadataKeys()) { logic.getMetadata(key).mergeIn(builder.logic.getMetadata(key)); } + return logic; } else { throw new Error('Unknown LogicNodeBuilder type'); } - return logic; } From 8a0a1b80bb8418866bcc2f68aaaa2b2e5bb055da Mon Sep 17 00:00:00 2001 From: Miles Malerba Date: Tue, 13 May 2025 22:34:42 +0000 Subject: [PATCH 4/4] refactor: defer predicate list expansion until merge time --- .../forms/experimental/src/logic_node_2.ts | 151 +++++++++--------- .../experimental/test/logic_node_2.spec.ts | 50 ++++++ 2 files changed, 129 insertions(+), 72 deletions(-) diff --git a/packages/forms/experimental/src/logic_node_2.ts b/packages/forms/experimental/src/logic_node_2.ts index 2fc651e0fc45..2bb735a5669e 100644 --- a/packages/forms/experimental/src/logic_node_2.ts +++ b/packages/forms/experimental/src/logic_node_2.ts @@ -8,17 +8,14 @@ import { } from './logic_node'; abstract class AbstractLogicNodeBuilder { - constructor(readonly predicates: ReadonlyArray) {} - abstract addHiddenRule(logic: LogicFn): void; abstract addDisabledRule(logic: LogicFn): void; abstract addErrorRule(logic: LogicFn): void; abstract addMetadataRule(key: MetadataKey, logic: LogicFn): void; abstract getChild(key: PropertyKey): LogicNodeBuilder; - abstract predicate(predicates: ReadonlyArray): AbstractLogicNodeBuilder; build(): LogicNode { - return new LogicNode(this); + return new LeafLogicNode(this, []); } } @@ -27,11 +24,7 @@ abstract class AbstractLogicNodeBuilder { */ export class LogicNodeBuilder extends AbstractLogicNodeBuilder { private current: NonMergableLogicNodeBuilder | undefined; - readonly all: AbstractLogicNodeBuilder[] = []; - - constructor(predicates: ReadonlyArray) { - super(predicates); - } + readonly all: {builder: AbstractLogicNodeBuilder; predicate?: Predicate}[] = []; addHiddenRule(logic: LogicFn): void { this.getCurrent().addHiddenRule(logic); @@ -53,27 +46,15 @@ export class LogicNodeBuilder extends AbstractLogicNodeBuilder { return this.getCurrent().getChild(key); } - predicate(predicates: ReadonlyArray) { - const newPredicates = [...this.predicates, ...predicates]; - const clone = new LogicNodeBuilder(newPredicates); - clone.all.push(...this.all.map((b) => b.predicate(newPredicates))); - clone.current = this.current?.predicate(newPredicates); - return clone; - } - mergeIn(other: LogicNodeBuilder, predicate?: Predicate): void { // Add the other builder to our collection, we'll defer the actual merging of the logic until // the logic node is requested to be created. In order to preserve the original ordering of the // rules, we close off the current builder to any further edits. If additional logic is added, // a new current builder will be created to capture it. - const predicates = [...this.predicates]; if (predicate) { - predicates.push(predicate); - } - if (predicates.length !== 0) { - this.all.push(other.predicate(predicates)); + this.all.push({builder: other, predicate}); } else { - this.all.push(other); + this.all.push({builder: other}); } this.current = undefined; } @@ -83,14 +64,14 @@ export class LogicNodeBuilder extends AbstractLogicNodeBuilder { // builder, a new one is created. In order to preserve the original ordering of the rules, we // clear the current builder whenever a separate builder tree is merged in. if (this.current === undefined) { - this.current = new NonMergableLogicNodeBuilder(this.predicates); - this.all.push(this.current); + this.current = new NonMergableLogicNodeBuilder(); + this.all.push({builder: this.current}); } return this.current; } static newRoot(): LogicNodeBuilder { - return new LogicNodeBuilder([]); + return new LogicNodeBuilder(); } } @@ -99,7 +80,7 @@ export class LogicNodeBuilder extends AbstractLogicNodeBuilder { * chunks of logic that do not require merging in other builders. */ class NonMergableLogicNodeBuilder extends AbstractLogicNodeBuilder { - logic = new Logic([]); + readonly logic = new Logic([]); readonly children = new Map(); override addHiddenRule(logic: LogicFn): void { @@ -118,19 +99,9 @@ class NonMergableLogicNodeBuilder extends AbstractLogicNodeBuilder { this.logic.getMetadata(key).push(logic); } - override predicate(predicates: ReadonlyArray) { - const newPredicates = [...this.predicates, ...predicates]; - const clone = new NonMergableLogicNodeBuilder(newPredicates); - for (const [prop, child] of this.children) { - clone.children.set(prop, child.predicate(newPredicates)); - } - clone.logic = this.logic; - return clone; - } - override getChild(key: PropertyKey): LogicNodeBuilder { if (!this.children.has(key)) { - this.children.set(key, new LogicNodeBuilder(this.predicates)); + this.children.set(key, new LogicNodeBuilder()); } return this.children.get(key)!; } @@ -170,46 +141,88 @@ export class Logic { getMetadataKeys() { return this.metadata.keys(); } + + mergeIn(other: Logic) { + this.disabled.mergeIn(other.disabled); + this.hidden.mergeIn(other.hidden); + this.errors.mergeIn(other.errors); + for (const key of other.getMetadataKeys()) { + this.getMetadata(key).mergeIn(other.getMetadata(key)); + } + } +} + +export interface LogicNode { + readonly logic: Logic; + getChild(key: PropertyKey): LogicNode; } /** * A tree structure of `Logic` corresponding to a tree of fields. */ -export class LogicNode { +class LeafLogicNode implements LogicNode { readonly logic: Logic; - constructor(private builder: AbstractLogicNodeBuilder | undefined) { - this.logic = builder ? createLogic(builder) : new Logic([]); + constructor( + private builder: AbstractLogicNodeBuilder | undefined, + private predicates: Predicate[], + ) { + this.logic = builder ? createLogic(builder, predicates) : new Logic([]); } // TODO: cache here, or just rely on the user of this API to do caching? getChild(key: PropertyKey): LogicNode { // The logic for a particular child may be spread across multiple builders. We lazily combine // this logic at the time the child logic node is requested to be created. - const childBuilders = this.builder ? getAllChildren(this.builder, key) : []; + const childBuilders = this.builder ? getAllChildBuilders(this.builder, key) : []; if (childBuilders.length <= 1) { - return new LogicNode(childBuilders[0]); + const {builder, predicates} = childBuilders[0]; + return new LeafLogicNode(builder, [...this.predicates, ...predicates]); } else { - // If there are multiple child builders, combine them all into one we ca pass to the new logic - // node. - const combined = LogicNodeBuilder.newRoot(); - for (const child of childBuilders) { - combined.mergeIn(child); - } - return new LogicNode(combined); + const builtNodes = childBuilders.map( + ({builder, predicates}) => new LeafLogicNode(builder, [...this.predicates, ...predicates]), + ); + return new CompositeLogicNode(builtNodes); + } + } +} + +class CompositeLogicNode implements LogicNode { + readonly logic: Logic; + + constructor(private all: LogicNode[]) { + this.logic = new Logic([]); + for (const node of all) { + this.logic.mergeIn(node.logic); } } + + getChild(key: PropertyKey): LogicNode { + return new CompositeLogicNode(this.all.flatMap((child) => child.getChild(key))); + } } /** * Gets all of the builders that contribute logic to the given child of the parent builder. */ -function getAllChildren(builder: AbstractLogicNodeBuilder, key: PropertyKey): LogicNodeBuilder[] { +function getAllChildBuilders( + builder: AbstractLogicNodeBuilder, + key: PropertyKey, +): {builder: LogicNodeBuilder; predicates: Predicate[]}[] { if (builder instanceof LogicNodeBuilder) { - return builder.all.flatMap((node) => getAllChildren(node, key)); + return builder.all.flatMap(({builder, predicate}) => { + const children = getAllChildBuilders(builder, key); + if (predicate) { + return children.map(({builder, predicates}) => ({ + builder, + predicates: [...predicates, predicate], + })); + } + return children; + }); } else if (builder instanceof NonMergableLogicNodeBuilder) { if (builder.children.has(key)) { - return [builder.children.get(key)!]; + return [{builder: builder.children.get(key)!, predicates: []}]; } } else { throw new Error('Unknown LogicNodeBuilder type'); @@ -220,30 +233,24 @@ function getAllChildren(builder: AbstractLogicNodeBuilder, key: PropertyKey): Lo /** * Creates the full `Logic` for a given builder. */ -function createLogic(builder: AbstractLogicNodeBuilder): Logic { +function createLogic(builder: AbstractLogicNodeBuilder, predicates: Predicate[]): Logic { + const logic = new Logic(predicates); if (builder instanceof LogicNodeBuilder) { - // Build the logic for all sub-builders, and merge them into one. - const logic = new Logic([]); - const builtNodes = builder.all.map((b) => b.build()); + // TODO: do we need to bind predicate to a specific field here? + // Specifically I think we need to split the idea of a predicate in the LogicNodeBuilder from + // the idea of a predicate in the LogicNode. Instead of a path, the LogicNode version should + // have a field context. + const builtNodes = builder.all.map( + ({builder, predicate}) => + new LeafLogicNode(builder, predicate ? [...predicates, predicate] : predicates), + ); for (const node of builtNodes) { - logic.disabled.mergeIn(node.logic.disabled); - logic.hidden.mergeIn(node.logic.hidden); - logic.errors.mergeIn(node.logic.errors); - for (const key of node.logic.getMetadataKeys()) { - logic.getMetadata(key).mergeIn(node.logic.getMetadata(key)); - } + logic.mergeIn(node.logic); } - return logic; } else if (builder instanceof NonMergableLogicNodeBuilder) { - const logic = new Logic(builder.predicates); - logic.disabled.mergeIn(builder.logic.disabled); - logic.hidden.mergeIn(builder.logic.hidden); - logic.errors.mergeIn(builder.logic.errors); - for (const key of builder.logic.getMetadataKeys()) { - logic.getMetadata(key).mergeIn(builder.logic.getMetadata(key)); - } - return logic; + logic.mergeIn(builder.logic); } else { throw new Error('Unknown LogicNodeBuilder type'); } + return logic; } diff --git a/packages/forms/experimental/test/logic_node_2.spec.ts b/packages/forms/experimental/test/logic_node_2.spec.ts index 56654fd2f7e3..a0a4ba97be14 100644 --- a/packages/forms/experimental/test/logic_node_2.spec.ts +++ b/packages/forms/experimental/test/logic_node_2.spec.ts @@ -343,4 +343,54 @@ describe('LogicNodeBuilder', () => { {kind: 'err-3'}, ]); }); + + it('should support circular logic structures', () => { + // const s = schema((p) => { + // validate(p, () => ({kind: 'err-1'})), + // apply(p.next, s); + // })); + + const builder = LogicNodeBuilder.newRoot(); + builder.addErrorRule(() => [{kind: 'err-1'}]); + builder.getChild('next').mergeIn(builder); + + const logicNode = builder.build(); + expect(logicNode.logic.errors.compute(fakeFieldContext)).toEqual([{kind: 'err-1'}]); + expect(logicNode.getChild('next').logic.errors.compute(fakeFieldContext)).toEqual([ + {kind: 'err-1'}, + ]); + expect( + logicNode.getChild('next').getChild('next').logic.errors.compute(fakeFieldContext), + ).toEqual([{kind: 'err-1'}]); + }); + + it('should support circular logic structures with predicate', () => { + // const s = schema((p) => { + // validate(p, () => ({kind: 'err-1'})), + // applyWhen(p.next, pred, s); + // })); + + const pred = signal(true); + const builder = LogicNodeBuilder.newRoot(); + builder.addErrorRule(() => [{kind: 'err-1'}]); + builder.getChild('next').mergeIn(builder, {fn: pred, path: undefined!}); + + const logicNode = builder.build(); + expect(logicNode.logic.errors.compute(fakeFieldContext)).toEqual([{kind: 'err-1'}]); + expect(logicNode.getChild('next').logic.errors.compute(fakeFieldContext)).toEqual([ + {kind: 'err-1'}, + ]); + expect( + logicNode.getChild('next').getChild('next').logic.errors.compute(fakeFieldContext), + ).toEqual([{kind: 'err-1'}]); + + // TODO: test that verifies that the same predicate can resolve with a different field context + // on `.next` vs on `.next.next` + pred.set(false); + expect(logicNode.logic.errors.compute(fakeFieldContext)).toEqual([{kind: 'err-1'}]); + expect(logicNode.getChild('next').logic.errors.compute(fakeFieldContext)).toEqual([]); + expect( + logicNode.getChild('next').getChild('next').logic.errors.compute(fakeFieldContext), + ).toEqual([]); + }); });