From 55c4a95b8649e8abf24c9f03212dd58f0a366833 Mon Sep 17 00:00:00 2001 From: Rob Eisenberg Date: Mon, 16 May 2022 23:09:44 -0400 Subject: [PATCH 1/9] feat: add two-way binding --- .../fast-element/docs/api-report.md | 23 ++++ .../fast-element/src/templating/binding.ts | 112 ++++++++++++++++++ 2 files changed, 135 insertions(+) diff --git a/packages/web-components/fast-element/docs/api-report.md b/packages/web-components/fast-element/docs/api-report.md index 7e064f29149..5084041f3c2 100644 --- a/packages/web-components/fast-element/docs/api-report.md +++ b/packages/web-components/fast-element/docs/api-report.md @@ -284,6 +284,12 @@ export type DecoratorAttributeConfiguration = Omit any; +}; + // @public export const DOM: Readonly<{ queueUpdate: (callable: Callable) => void; @@ -813,6 +819,23 @@ export type TrustedTypesPolicy = { createHTML(html: string): string; }; +// @public +export const twoWay: BindingConfig & BindingConfigResolver; + +// @public +export class TwoWayBinding extends ChangeBinding { + bind(source: any, context: ExecutionContext, targets: ViewBehaviorTargets): void; + static configure(settings: TwoWaySettings): void; + // @internal (undocumented) + handleEvent(event: Event): void; + unbind(source: any, context: ExecutionContext, targets: ViewBehaviorTargets): void; +} + +// @public +export interface TwoWaySettings { + determineChangeEvent(directive: HTMLBindingDirective, target: HTMLElement): string; +} + // Warning: (ae-internal-missing-underscore) The name "TypeDefinition" should be prefixed with an underscore because the declaration is marked as @internal // // @internal diff --git a/packages/web-components/fast-element/src/templating/binding.ts b/packages/web-components/fast-element/src/templating/binding.ts index b271586656e..6ab7a25de61 100644 --- a/packages/web-components/fast-element/src/templating/binding.ts +++ b/packages/web-components/fast-element/src/templating/binding.ts @@ -4,6 +4,7 @@ import { BindingObserver, ExecutionContext, Observable, + ObservationRecord, } from "../observation/observable.js"; import { FAST } from "../platform.js"; import { DOM } from "./dom.js"; @@ -593,6 +594,100 @@ export class EventBinding { } } +/** + * The settings required to enable two-way binding. + * @public + */ +export interface TwoWaySettings { + /** + * Determines which event to listen to, to detect changes in the view. + * @param directive - The directive to determine the change event for. + * @param target - The target element to determine the change event for. + */ + determineChangeEvent(directive: HTMLBindingDirective, target: HTMLElement): string; +} + +let twoWaySettings: TwoWaySettings = { + determineChangeEvent() { + return "change"; + }, +}; + +/** + * A binding behavior for bindings that update in two directions. + * @public + */ +export class TwoWayBinding extends ChangeBinding { + private changeEvent: string; + + /** + * Bind this behavior to the source. + * @param source - The source to bind to. + * @param context - The execution context that the binding is operating within. + * @param targets - The targets that behaviors in a view can attach to. + */ + bind(source: any, context: ExecutionContext, targets: ViewBehaviorTargets): void { + super.bind(source, context, targets); + + const directive = this.directive; + const target = targets[directive.nodeId] as HTMLElement; + + if (!this.changeEvent) { + this.changeEvent = + directive.options.changeEvent ?? + twoWaySettings.determineChangeEvent(directive, target); + } + + target.addEventListener(this.changeEvent, this); + } + + /** + * Unbinds this behavior from the source. + * @param source - The source to unbind from. + * @param context - The execution context that the binding is operating within. + * @param targets - The targets that behaviors in a view can attach to. + */ + unbind(source: any, context: ExecutionContext, targets: ViewBehaviorTargets): void { + super.unbind(source, context, targets); + (targets[this.directive.nodeId] as HTMLElement).removeEventListener( + this.changeEvent, + this + ); + } + + /** @internal */ + public handleEvent(event: Event): void { + const directive = this.directive; + const target = event.currentTarget as HTMLElement; + const observer: BindingObserver = target[directive.id]; + + let value; + + switch (directive.aspectType) { + case 1: + value = target.getAttribute(directive.targetAspect); + break; + case 2: + value = target.hasAttribute(directive.targetAspect); + break; + default: + value = target[directive.targetAspect]; + break; + } + + const last = (observer as any).last as ObservationRecord; // using internal API!!! + last.propertySource[last.propertyName] = directive.options.fromView(value); + } + + /** + * Configures two-way binding. + * @param settings - The settings to use for the two-way binding system. + */ + public static configure(settings: TwoWaySettings) { + twoWaySettings = settings; + } +} + /** * The default binding options. * @public @@ -608,6 +703,23 @@ export const onChange = BindingConfig.define( {} as DefaultBindingOptions ); +/** + * The default twoWay binding options. + * @public + */ +export type DefaultTwoWayBindingOptions = DefaultBindingOptions & { + changeEvent?: string; + fromView?: (value: any) => any; +}; + +/** + * The default twoWay binding configuration. + * @public + */ +export const twoWay = BindingConfig.define(BindingMode.define(TwoWayBinding), { + fromView: v => v, +} as DefaultTwoWayBindingOptions); + /** * The default onTime binding configuration. * @public From d0efb36516c79910103cdffc37bb4edce2c9c369 Mon Sep 17 00:00:00 2001 From: Rob Eisenberg Date: Tue, 17 May 2022 11:37:26 -0400 Subject: [PATCH 2/9] test: add tests for previously created binding modes --- .../src/templating/binding.spec.ts | 203 +++++++++++++++++- 1 file changed, 201 insertions(+), 2 deletions(-) diff --git a/packages/web-components/fast-element/src/templating/binding.spec.ts b/packages/web-components/fast-element/src/templating/binding.spec.ts index 5969df7f4ea..e5afc0ed919 100644 --- a/packages/web-components/fast-element/src/templating/binding.spec.ts +++ b/packages/web-components/fast-element/src/templating/binding.spec.ts @@ -1,12 +1,13 @@ import { expect } from "chai"; -import { bind, HTMLBindingDirective } from "./binding"; +import { bind, BindingConfig, BindingMode, HTMLBindingDirective, onChange, oneTime, signal, SignalBinding } from "./binding"; import { ExecutionContext, observable } from "../observation/observable"; import { html, ViewTemplate } from "./template"; import { toHTML } from "../__test__/helpers"; import { SyntheticView, HTMLView } from "./view"; import { Updates } from "../observation/update-queue"; +import { Aspect, AspectType } from "./html-directive"; -describe("The HTML binding directive", () => { +describe.only("The HTML binding directive", () => { class Model { constructor(value: any) { this.value = value; @@ -41,6 +42,25 @@ describe("The HTML binding directive", () => { return { directive, behavior, node, parentNode, targets }; } + function bindingWithConfig(config: BindingConfig, sourceAspect?: string) { + const directive = bind(x => x.value, config) as HTMLBindingDirective; + directive.nodeId = 'r'; + + if (sourceAspect) { + Aspect.assign(directive, sourceAspect); + } + + const node = document.createElement("div"); + const targets = { r: node }; + + const behavior = directive.createBehavior(targets); + const parentNode = document.createElement("div"); + + parentNode.appendChild(node); + + return { directive, behavior, node, parentNode, targets }; + } + context("when binding text content", () => { it("initially sets the text of a node", () => { const { behavior, node, targets } = contentBinding(); @@ -249,4 +269,183 @@ describe("The HTML binding directive", () => { expect(toHTML(parentNode)).to.equal(`This is a template. value`); }); }); + + const aspectScenarios = [ + { + name: "content", + sourceAspect: "", + originalValue: "This is a test", + newValue: "This is another test", + getValue(node: HTMLElement) { + return node.textContent; + } + }, + { + name: "attribute", + sourceAspect: "test-attribute", + originalValue: "This is a test", + newValue: "This is another test", + getValue(node: HTMLElement) { + return node.getAttribute("test-attribute"); + } + }, + { + name: "boolean attribute", + sourceAspect: "?test-boolean-attribute", + originalValue: true, + newValue: false, + getValue(node: HTMLElement) { + return node.hasAttribute("test-boolean-attribute"); + } + }, + { + name: "property", + sourceAspect: ":testProperty", + originalValue: "This is a test", + newValue: "This is another test", + getValue(node: HTMLElement) { + return (node as any).testProperty; + } + }, + ]; + + context("when binding on-change", () => { + for (const aspectScenario of aspectScenarios) { + it(`sets the initial value of a ${aspectScenario.name} binding`, () => { + const { behavior, node, targets } = bindingWithConfig(onChange, aspectScenario.sourceAspect); + const model = new Model(aspectScenario.originalValue); + + behavior.bind(model, ExecutionContext.default, targets); + + expect(aspectScenario.getValue(node)).to.equal(model.value); + }); + + it(`updates the ${aspectScenario.name} when the model changes`, async () => { + const { behavior, node, targets } = bindingWithConfig(onChange, aspectScenario.sourceAspect); + const model = new Model(aspectScenario.originalValue); + + behavior.bind(model, ExecutionContext.default, targets); + + expect(aspectScenario.getValue(node)).to.equal(model.value); + + model.value = aspectScenario.newValue; + + await Updates.next(); + + expect(aspectScenario.getValue(node)).to.equal(model.value); + }); + + it(`doesn't update the ${aspectScenario.name} after unbind`, async () => { + const { behavior, node, targets } = bindingWithConfig(onChange, aspectScenario.sourceAspect); + const model = new Model(aspectScenario.originalValue); + + behavior.bind(model, ExecutionContext.default, targets); + + expect(aspectScenario.getValue(node)).to.equal(model.value); + + behavior.unbind(model, ExecutionContext.default, targets) ; + model.value = aspectScenario.newValue; + + await Updates.next(); + + expect(aspectScenario.getValue(node)).to.equal(aspectScenario.originalValue); + }); + } + }); + + context("when binding one-time", () => { + for (const aspectScenario of aspectScenarios) { + it(`sets the initial value of a ${aspectScenario.name} binding`, () => { + const { behavior, node, targets } = bindingWithConfig(oneTime, aspectScenario.sourceAspect); + const model = new Model(aspectScenario.originalValue); + + behavior.bind(model, ExecutionContext.default, targets); + + expect(aspectScenario.getValue(node)).to.equal(model.value); + }); + + it(`does not update the ${aspectScenario.name} after the initial set`, async () => { + const { behavior, node, targets } = bindingWithConfig(oneTime, aspectScenario.sourceAspect); + const model = new Model(aspectScenario.originalValue); + + behavior.bind(model, ExecutionContext.default, targets); + + expect(aspectScenario.getValue(node)).to.equal(aspectScenario.originalValue); + + model.value = aspectScenario.newValue; + + await Updates.next(); + + expect(aspectScenario.getValue(node)).to.equal(aspectScenario.originalValue); + }); + + it(`doesn't update the ${aspectScenario.name} after unbind`, async () => { + const { behavior, node, targets } = bindingWithConfig(oneTime, aspectScenario.sourceAspect); + const model = new Model(aspectScenario.originalValue); + + behavior.bind(model, ExecutionContext.default, targets); + + expect(aspectScenario.getValue(node)).to.equal(aspectScenario.originalValue); + + behavior.unbind(model, ExecutionContext.default, targets); + model.value = aspectScenario.newValue; + await Updates.next(); + + expect(aspectScenario.getValue(node)).to.equal(aspectScenario.originalValue); + }); + } + }); + + context("when binding with a signal", () => { + for (const aspectScenario of aspectScenarios) { + it(`sets the initial value of the ${aspectScenario.name} binding`, () => { + const { behavior, node, targets } = bindingWithConfig(signal("test-signal"), aspectScenario.sourceAspect); + const model = new Model(aspectScenario.originalValue); + + behavior.bind(model, ExecutionContext.default, targets); + + expect(aspectScenario.getValue(node)).to.equal(model.value); + }); + + it(`updates the ${aspectScenario.name} only when the signal is sent`, async () => { + const signalName = "test-signal"; + const { behavior, node, targets } = bindingWithConfig(signal(signalName), aspectScenario.sourceAspect); + const model = new Model(aspectScenario.originalValue); + + behavior.bind(model, ExecutionContext.default, targets); + + expect(aspectScenario.getValue(node)).to.equal(aspectScenario.originalValue); + + model.value = aspectScenario.newValue; + + await Updates.next(); + + expect(aspectScenario.getValue(node)).to.equal(aspectScenario.originalValue); + + SignalBinding.send(signalName); + + await Updates.next(); + + expect(aspectScenario.getValue(node)).to.equal(model.value); + }); + + it(`doesn't respond to signals for a ${aspectScenario.name} binding after unbind`, async () => { + const signalName = "test-signal"; + const { behavior, node, targets } = bindingWithConfig(signal(signalName), aspectScenario.sourceAspect); + const model = new Model(aspectScenario.originalValue); + + behavior.bind(model, ExecutionContext.default, targets); + + expect(aspectScenario.getValue(node)).to.equal(model.value); + + behavior.unbind(model, ExecutionContext.default, targets); + model.value = aspectScenario.newValue; + SignalBinding.send(signalName); + + await Updates.next(); + + expect(aspectScenario.getValue(node)).to.equal(aspectScenario.originalValue); + }); + } + }); }); From 10a2e45aa8724f03596f940207fc466ff2f14483 Mon Sep 17 00:00:00 2001 From: Rob Eisenberg Date: Tue, 17 May 2022 12:05:08 -0400 Subject: [PATCH 3/9] test: add tests for two-way bindings --- .../src/templating/binding.spec.ts | 95 ++++++++++++++++++- .../fast-element/src/templating/binding.ts | 3 + 2 files changed, 95 insertions(+), 3 deletions(-) diff --git a/packages/web-components/fast-element/src/templating/binding.spec.ts b/packages/web-components/fast-element/src/templating/binding.spec.ts index e5afc0ed919..ab7fc431d15 100644 --- a/packages/web-components/fast-element/src/templating/binding.spec.ts +++ b/packages/web-components/fast-element/src/templating/binding.spec.ts @@ -1,13 +1,14 @@ import { expect } from "chai"; -import { bind, BindingConfig, BindingMode, HTMLBindingDirective, onChange, oneTime, signal, SignalBinding } from "./binding"; +import { bind, BindingConfig, BindingMode, HTMLBindingDirective, onChange, oneTime, signal, SignalBinding, twoWay } from "./binding"; import { ExecutionContext, observable } from "../observation/observable"; import { html, ViewTemplate } from "./template"; import { toHTML } from "../__test__/helpers"; import { SyntheticView, HTMLView } from "./view"; import { Updates } from "../observation/update-queue"; -import { Aspect, AspectType } from "./html-directive"; +import { Aspect } from "./html-directive"; +import { DOM } from "./dom"; -describe.only("The HTML binding directive", () => { +describe("The HTML binding directive", () => { class Model { constructor(value: any) { this.value = value; @@ -278,6 +279,9 @@ describe.only("The HTML binding directive", () => { newValue: "This is another test", getValue(node: HTMLElement) { return node.textContent; + }, + setValue(node: HTMLElement, value: any) { + node.textContent = value; } }, { @@ -287,6 +291,9 @@ describe.only("The HTML binding directive", () => { newValue: "This is another test", getValue(node: HTMLElement) { return node.getAttribute("test-attribute"); + }, + setValue(node: HTMLElement, value: any) { + DOM.setAttribute(node, "test-attribute", value); } }, { @@ -296,6 +303,9 @@ describe.only("The HTML binding directive", () => { newValue: false, getValue(node: HTMLElement) { return node.hasAttribute("test-boolean-attribute"); + }, + setValue(node: HTMLElement, value: boolean) { + DOM.setBooleanAttribute(node, "test-boolean-attribute", value); } }, { @@ -305,6 +315,9 @@ describe.only("The HTML binding directive", () => { newValue: "This is another test", getValue(node: HTMLElement) { return (node as any).testProperty; + }, + setValue(node: HTMLElement, value: any) { + (node as any).testProperty = value; } }, ]; @@ -448,4 +461,80 @@ describe.only("The HTML binding directive", () => { }); } }); + + context("when binding two-way", () => { + for (const aspectScenario of aspectScenarios) { + it(`sets the initial value of the ${aspectScenario.name} binding`, () => { + const { behavior, node, targets } = bindingWithConfig(twoWay, aspectScenario.sourceAspect); + const model = new Model(aspectScenario.originalValue); + + behavior.bind(model, ExecutionContext.default, targets); + + expect(aspectScenario.getValue(node)).to.equal(model.value); + }); + + it(`updates the ${aspectScenario.name} when the model changes`, async () => { + const { behavior, node, targets } = bindingWithConfig(twoWay, aspectScenario.sourceAspect); + const model = new Model(aspectScenario.originalValue); + + behavior.bind(model, ExecutionContext.default, targets); + + expect(aspectScenario.getValue(node)).to.equal(aspectScenario.originalValue); + + model.value = aspectScenario.newValue; + + await Updates.next(); + + expect(aspectScenario.getValue(node)).to.equal(model.value); + }); + + it(`updates the model when a change event fires for the ${aspectScenario.name}`, async () => { + const { behavior, node, targets } = bindingWithConfig(twoWay, aspectScenario.sourceAspect); + const model = new Model(aspectScenario.originalValue); + + behavior.bind(model, ExecutionContext.default, targets); + + expect(aspectScenario.getValue(node)).to.equal(aspectScenario.originalValue); + + aspectScenario.setValue(node, aspectScenario.newValue as boolean); + node.dispatchEvent(new CustomEvent("change")); + + await Updates.next(); + + expect(model.value).to.equal(aspectScenario.newValue); + }); + + it(`updates the model when a configured event fires for the ${aspectScenario.name}`, async () => { + const changeEvent = "foo"; + const { behavior, node, targets } = bindingWithConfig(twoWay({changeEvent}), aspectScenario.sourceAspect); + const model = new Model(aspectScenario.originalValue); + + behavior.bind(model, ExecutionContext.default, targets); + + expect(aspectScenario.getValue(node)).to.equal(aspectScenario.originalValue); + + aspectScenario.setValue(node, aspectScenario.newValue as boolean); + node.dispatchEvent(new CustomEvent(changeEvent)); + + await Updates.next(); + + expect(model.value).to.equal(aspectScenario.newValue); + }); + + it(`doesn't update the ${aspectScenario.name} after unbind`, async () => { + const { behavior, node, targets } = bindingWithConfig(twoWay, aspectScenario.sourceAspect); + const model = new Model(aspectScenario.originalValue); + + behavior.bind(model, ExecutionContext.default, targets); + + expect(aspectScenario.getValue(node)).to.equal(model.value); + + behavior.unbind(model, ExecutionContext.default, targets); + model.value = aspectScenario.newValue; + await Updates.next(); + + expect(aspectScenario.getValue(node)).to.equal(aspectScenario.originalValue); + }); + } + }); }); diff --git a/packages/web-components/fast-element/src/templating/binding.ts b/packages/web-components/fast-element/src/templating/binding.ts index 6ab7a25de61..e88e8d8b635 100644 --- a/packages/web-components/fast-element/src/templating/binding.ts +++ b/packages/web-components/fast-element/src/templating/binding.ts @@ -670,6 +670,9 @@ export class TwoWayBinding extends ChangeBinding { case 2: value = target.hasAttribute(directive.targetAspect); break; + case 4: + value = target.innerText; + break; default: value = target[directive.targetAspect]; break; From cc251427f0ce0646d1ccc0b5f50138afc1a5f146 Mon Sep 17 00:00:00 2001 From: Rob Eisenberg Date: Tue, 17 May 2022 12:23:47 -0400 Subject: [PATCH 4/9] test: add event binding tests --- .../src/templating/binding.spec.ts | 102 +++++++++++++++++- 1 file changed, 99 insertions(+), 3 deletions(-) diff --git a/packages/web-components/fast-element/src/templating/binding.spec.ts b/packages/web-components/fast-element/src/templating/binding.spec.ts index ab7fc431d15..422c5a257e1 100644 --- a/packages/web-components/fast-element/src/templating/binding.spec.ts +++ b/packages/web-components/fast-element/src/templating/binding.spec.ts @@ -17,11 +17,16 @@ describe("The HTML binding directive", () => { @observable value: any = null; @observable private trigger = 0; @observable knownValue = "value"; + actionInvokeCount = 0; forceComputedUpdate() { this.trigger++; } + invokeAction() { + this.actionInvokeCount++; + } + get computedValue() { const trigger = this.trigger; return this.value; @@ -62,6 +67,23 @@ describe("The HTML binding directive", () => { return { directive, behavior, node, parentNode, targets }; } + function eventBinding(config: BindingConfig, sourceAspect: string) { + const directive = bind(x => x.invokeAction(), config) as HTMLBindingDirective; + directive.nodeId = 'r'; + + Aspect.assign(directive, sourceAspect); + + const node = document.createElement("div"); + const targets = { r: node }; + + const behavior = directive.createBehavior(targets); + const parentNode = document.createElement("div"); + + parentNode.appendChild(node); + + return { directive, behavior, node, parentNode, targets }; + } + context("when binding text content", () => { it("initially sets the text of a node", () => { const { behavior, node, targets } = contentBinding(); @@ -304,7 +326,7 @@ describe("The HTML binding directive", () => { getValue(node: HTMLElement) { return node.hasAttribute("test-boolean-attribute"); }, - setValue(node: HTMLElement, value: boolean) { + setValue(node: HTMLElement, value: any) { DOM.setBooleanAttribute(node, "test-boolean-attribute", value); } }, @@ -496,7 +518,7 @@ describe("The HTML binding directive", () => { expect(aspectScenario.getValue(node)).to.equal(aspectScenario.originalValue); - aspectScenario.setValue(node, aspectScenario.newValue as boolean); + aspectScenario.setValue(node, aspectScenario.newValue); node.dispatchEvent(new CustomEvent("change")); await Updates.next(); @@ -504,6 +526,23 @@ describe("The HTML binding directive", () => { expect(model.value).to.equal(aspectScenario.newValue); }); + it(`updates the model when a change event fires for the ${aspectScenario.name} with conversion`, async () => { + const fromView = value => "fixed value"; + const { behavior, node, targets } = bindingWithConfig(twoWay({ fromView }), aspectScenario.sourceAspect); + const model = new Model(aspectScenario.originalValue); + + behavior.bind(model, ExecutionContext.default, targets); + + expect(aspectScenario.getValue(node)).to.equal(aspectScenario.originalValue); + + aspectScenario.setValue(node, aspectScenario.newValue); + node.dispatchEvent(new CustomEvent("change")); + + await Updates.next(); + + expect(model.value).to.equal("fixed value"); + }); + it(`updates the model when a configured event fires for the ${aspectScenario.name}`, async () => { const changeEvent = "foo"; const { behavior, node, targets } = bindingWithConfig(twoWay({changeEvent}), aspectScenario.sourceAspect); @@ -513,7 +552,7 @@ describe("The HTML binding directive", () => { expect(aspectScenario.getValue(node)).to.equal(aspectScenario.originalValue); - aspectScenario.setValue(node, aspectScenario.newValue as boolean); + aspectScenario.setValue(node, aspectScenario.newValue); node.dispatchEvent(new CustomEvent(changeEvent)); await Updates.next(); @@ -537,4 +576,61 @@ describe("The HTML binding directive", () => { }); } }); + + context("when binding events", () => { + it("does not invoke the method on bind", () => { + const { behavior, targets } = eventBinding(onChange, "@my-event"); + const model = new Model("Test value."); + + behavior.bind(model, ExecutionContext.default, targets); + expect(model.actionInvokeCount).to.equal(0); + }); + + it("invokes the method each time the event is raised", () => { + const { behavior, node, targets } = eventBinding(onChange, "@my-event"); + const model = new Model("Test value."); + + behavior.bind(model, ExecutionContext.default, targets); + expect(model.actionInvokeCount).to.equal(0); + + node.dispatchEvent(new CustomEvent("my-event")); + expect(model.actionInvokeCount).to.equal(1); + + node.dispatchEvent(new CustomEvent("my-event")); + expect(model.actionInvokeCount).to.equal(2); + + node.dispatchEvent(new CustomEvent("my-event")); + expect(model.actionInvokeCount).to.equal(3); + }); + + it("invokes the method one time for a one time event", () => { + const { behavior, node, targets } = eventBinding(oneTime, "@my-event"); + const model = new Model("Test value."); + + behavior.bind(model, ExecutionContext.default, targets); + expect(model.actionInvokeCount).to.equal(0); + + node.dispatchEvent(new CustomEvent("my-event")); + expect(model.actionInvokeCount).to.equal(1); + + node.dispatchEvent(new CustomEvent("my-event")); + expect(model.actionInvokeCount).to.equal(1); + }); + + it("does not invoke the method after unbind", () => { + const { behavior, node, targets } = eventBinding(onChange, "@my-event"); + const model = new Model("Test value."); + + behavior.bind(model, ExecutionContext.default, targets); + expect(model.actionInvokeCount).to.equal(0); + + node.dispatchEvent(new CustomEvent("my-event")); + expect(model.actionInvokeCount).to.equal(1); + + behavior.unbind(model, ExecutionContext.default, targets); + + node.dispatchEvent(new CustomEvent("my-event")); + expect(model.actionInvokeCount).to.equal(1); + }); + }); }); From 8a5c79540c8af666f6ac722817688901ed26bedc Mon Sep 17 00:00:00 2001 From: EisenbergEffect Date: Tue, 17 May 2022 12:27:28 -0400 Subject: [PATCH 5/9] Change files --- ...-fast-element-812c3a96-4de5-4429-907a-86c354742b82.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 change/@microsoft-fast-element-812c3a96-4de5-4429-907a-86c354742b82.json diff --git a/change/@microsoft-fast-element-812c3a96-4de5-4429-907a-86c354742b82.json b/change/@microsoft-fast-element-812c3a96-4de5-4429-907a-86c354742b82.json new file mode 100644 index 00000000000..02ee907948f --- /dev/null +++ b/change/@microsoft-fast-element-812c3a96-4de5-4429-907a-86c354742b82.json @@ -0,0 +1,7 @@ +{ + "type": "minor", + "comment": "feat: add two-way binding", + "packageName": "@microsoft/fast-element", + "email": "roeisenb@microsoft.com", + "dependentChangeType": "patch" +} From fca40ebd61138547e116981ec467ba558bddcde5 Mon Sep 17 00:00:00 2001 From: Rob Eisenberg Date: Tue, 17 May 2022 15:50:58 -0400 Subject: [PATCH 6/9] fix: prevent memory leek in one-time events --- .../web-components/fast-element/src/templating/binding.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/web-components/fast-element/src/templating/binding.ts b/packages/web-components/fast-element/src/templating/binding.ts index e88e8d8b635..f0efbeaf641 100644 --- a/packages/web-components/fast-element/src/templating/binding.ts +++ b/packages/web-components/fast-element/src/templating/binding.ts @@ -591,6 +591,10 @@ export class EventBinding { if (result !== true) { event.preventDefault(); } + + if (this.directive.options.once) { + target.$fastSource = target.$fastContext = null; + } } } From 1165d96dbdd1dcec935852ff6a074e479f1bf0d3 Mon Sep 17 00:00:00 2001 From: Rob Eisenberg Date: Tue, 17 May 2022 16:21:43 -0400 Subject: [PATCH 7/9] fix: prevent clashing state for behaviors on the same node --- .../fast-element/docs/api-report.md | 1 + .../fast-element/src/templating/binding.ts | 82 +++++++++++-------- 2 files changed, 47 insertions(+), 36 deletions(-) diff --git a/packages/web-components/fast-element/docs/api-report.md b/packages/web-components/fast-element/docs/api-report.md index 5084041f3c2..70897de6f01 100644 --- a/packages/web-components/fast-element/docs/api-report.md +++ b/packages/web-components/fast-element/docs/api-report.md @@ -148,6 +148,7 @@ export interface CaptureType { export class ChangeBinding extends UpdateBinding { constructor(directive: HTMLBindingDirective, updateTarget: UpdateTarget); bind(source: any, context: ExecutionContext, targets: ViewBehaviorTargets): void; + protected getObserver(target: Node): BindingObserver; // @internal (undocumented) handleChange(binding: Binding, observer: BindingObserver): void; unbind(source: any, context: ExecutionContext, targets: ViewBehaviorTargets): void; diff --git a/packages/web-components/fast-element/src/templating/binding.ts b/packages/web-components/fast-element/src/templating/binding.ts index f0efbeaf641..3d005c41bd3 100644 --- a/packages/web-components/fast-element/src/templating/binding.ts +++ b/packages/web-components/fast-element/src/templating/binding.ts @@ -454,6 +454,7 @@ export class SignalBinding extends UpdateBinding { */ export class ChangeBinding extends UpdateBinding { private isBindingVolatile: boolean; + private observerProperty: string; /** * Creates an instance of ChangeBinding. @@ -463,6 +464,23 @@ export class ChangeBinding extends UpdateBinding { constructor(directive: HTMLBindingDirective, updateTarget: UpdateTarget) { super(directive, updateTarget); this.isBindingVolatile = Observable.isVolatileBinding(directive.binding); + this.observerProperty = `${directive.id}-o`; + } + + /** + * Returns the binding observer used to update the node. + * @param target - The target node. + * @returns A BindingObserver. + */ + protected getObserver(target: Node): BindingObserver { + return ( + target[this.observerProperty] ?? + (target[this.observerProperty] = Observable.binding( + this.directive.binding, + this, + this.isBindingVolatile + )) + ); } /** @@ -474,13 +492,7 @@ export class ChangeBinding extends UpdateBinding { bind(source: any, context: ExecutionContext, targets: ViewBehaviorTargets): void { const directive = this.directive; const target = targets[directive.nodeId]; - const observer: BindingObserver = - target[directive.id] ?? - (target[directive.id] = Observable.binding( - directive.binding, - this, - this.isBindingVolatile - )); + const observer = this.getObserver(target); (observer as any).target = target; (observer as any).source = source; @@ -488,7 +500,7 @@ export class ChangeBinding extends UpdateBinding { this.updateTarget( target, - directive.targetAspect!, + directive.targetAspect, observer.observe(source, context), source, context @@ -503,11 +515,11 @@ export class ChangeBinding extends UpdateBinding { */ unbind(source: any, context: ExecutionContext, targets: ViewBehaviorTargets): void { const target = targets[this.directive.nodeId]; - const observer = target[this.directive.id]; + const observer = this.getObserver(target); observer.disconnect(); - observer.target = null; - observer.source = null; - observer.context = null; + (observer as any).target = null; + (observer as any).source = null; + (observer as any).context = null; } /** @internal */ @@ -517,7 +529,7 @@ export class ChangeBinding extends UpdateBinding { const context = (observer as any).context; this.updateTarget( target, - this.directive.targetAspect!, + this.directive.targetAspect, observer.observe(source, context!), source, context @@ -525,21 +537,22 @@ export class ChangeBinding extends UpdateBinding { } } -type FASTEventSource = Node & { - $fastSource: any; - $fastContext: ExecutionContext | null; -}; - /** * A binding behavior for handling events. * @public */ export class EventBinding { + private contextProperty: string; + private sourceProperty: string; + /** * Creates an instance of EventBinding. * @param directive - The directive that has the configuration for this behavior. */ - constructor(public readonly directive: HTMLBindingDirective) {} + constructor(public readonly directive: HTMLBindingDirective) { + this.sourceProperty = `${directive.id}-s`; + this.contextProperty = `${directive.id}-c`; + } /** * Bind this behavior to the source. @@ -549,9 +562,9 @@ export class EventBinding { */ bind(source: any, context: ExecutionContext, targets: ViewBehaviorTargets): void { const directive = this.directive; - const target = targets[directive.nodeId] as FASTEventSource; - target.$fastSource = source; - target.$fastContext = context; + const target = targets[directive.nodeId]; + target[this.sourceProperty] = source; + target[this.contextProperty] = context; target.addEventListener(directive.targetAspect, this, directive.options); } @@ -562,13 +575,10 @@ export class EventBinding { * @param targets - The targets that behaviors in a view can attach to. */ unbind(source: any, context: ExecutionContext, targets: ViewBehaviorTargets): void { - const target = targets[this.directive.nodeId] as FASTEventSource; - target.$fastSource = target.$fastContext = null; - target.removeEventListener( - this.directive.targetAspect, - this, - this.directive.options - ); + const directive = this.directive; + const target = targets[directive.nodeId]; + target[this.sourceProperty] = target[this.contextProperty] = null; + target.removeEventListener(directive.targetAspect, this, directive.options); } /** @@ -583,18 +593,18 @@ export class EventBinding { * @internal */ handleEvent(event: Event): void { - const target = event.currentTarget as FASTEventSource; + const target = event.currentTarget!; + ExecutionContext.setEvent(event); - const result = this.directive.binding(target.$fastSource, target.$fastContext!); + const result = this.directive.binding( + target[this.sourceProperty], + target[this.contextProperty] + ); ExecutionContext.setEvent(null); if (result !== true) { event.preventDefault(); } - - if (this.directive.options.once) { - target.$fastSource = target.$fastContext = null; - } } } @@ -663,7 +673,6 @@ export class TwoWayBinding extends ChangeBinding { public handleEvent(event: Event): void { const directive = this.directive; const target = event.currentTarget as HTMLElement; - const observer: BindingObserver = target[directive.id]; let value; @@ -682,6 +691,7 @@ export class TwoWayBinding extends ChangeBinding { break; } + const observer = this.getObserver(target); const last = (observer as any).last as ObservationRecord; // using internal API!!! last.propertySource[last.propertyName] = directive.options.fromView(value); } From 3a9fabc7400303799ec9a01627548bf78414a30f Mon Sep 17 00:00:00 2001 From: Rob Eisenberg Date: Tue, 17 May 2022 16:37:05 -0400 Subject: [PATCH 8/9] fix: more internal consistency for binding state --- .../web-components/fast-element/src/templating/binding.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/web-components/fast-element/src/templating/binding.ts b/packages/web-components/fast-element/src/templating/binding.ts index 3d005c41bd3..824d1cdca43 100644 --- a/packages/web-components/fast-element/src/templating/binding.ts +++ b/packages/web-components/fast-element/src/templating/binding.ts @@ -304,7 +304,7 @@ function updateTokenListTarget( value: any ): void { const directive = this.directive; - const lookup = `${directive.id}-token-list`; + const lookup = `${directive.id}-t`; const state: TokenListState = target[lookup] ?? (target[lookup] = { c: 0, v: Object.create(null) }); const versions = state.v; @@ -374,6 +374,8 @@ const signals: Record = Object.create * @public */ export class SignalBinding extends UpdateBinding { + private handlerProperty = `${this.directive.id}-h`; + /** * Bind this behavior to the source. * @param source - The source to bind to. @@ -384,7 +386,7 @@ export class SignalBinding extends UpdateBinding { const directive = this.directive; const target = targets[directive.nodeId]; const signal = this.getSignal(source, context); - const handler = (target[directive.id] = () => { + const handler = (target[this.handlerProperty] = () => { this.updateTarget( target, directive.targetAspect!, @@ -420,7 +422,7 @@ export class SignalBinding extends UpdateBinding { if (found && Array.isArray(found)) { const directive = this.directive; const target = targets[directive.nodeId]; - const handler = target[directive.id]; + const handler = target[this.handlerProperty]; const index = found.indexOf(handler); if (index !== -1) { found.splice(index, 1); From 2e919ad5c638dcb70a318cc1586e2dc728394297 Mon Sep 17 00:00:00 2001 From: Rob Eisenberg Date: Tue, 17 May 2022 16:49:17 -0400 Subject: [PATCH 9/9] fix: more consistency and prevent internal clobber of directive state --- .../fast-element/docs/api-report.md | 1 + .../fast-element/src/templating/children.ts | 16 +++++++++------- .../src/templating/node-observation.ts | 15 +++++++++++++-- .../fast-element/src/templating/slotted.ts | 3 +-- 4 files changed, 24 insertions(+), 11 deletions(-) diff --git a/packages/web-components/fast-element/docs/api-report.md b/packages/web-components/fast-element/docs/api-report.md index 70897de6f01..60e857fe552 100644 --- a/packages/web-components/fast-element/docs/api-report.md +++ b/packages/web-components/fast-element/docs/api-report.md @@ -540,6 +540,7 @@ export abstract class NodeObservationDirective ex protected computeNodes(target: any): Node[]; protected abstract disconnect(target: any): void; protected abstract getNodes(target: any): Node[]; + protected getSource(target: Node): any; protected abstract observe(target: any): void; unbind(source: any, context: ExecutionContext, targets: ViewBehaviorTargets): void; protected updateTarget(source: any, value: ReadonlyArray): void; diff --git a/packages/web-components/fast-element/src/templating/children.ts b/packages/web-components/fast-element/src/templating/children.ts index ea5767224e0..1e251570a2d 100644 --- a/packages/web-components/fast-element/src/templating/children.ts +++ b/packages/web-components/fast-element/src/templating/children.ts @@ -45,6 +45,8 @@ export type ChildrenDirectiveOptions = export class ChildrenDirective extends NodeObservationDirective< ChildrenDirectiveOptions > { + private observerProperty = `${this.id}-o`; + /** * Creates an instance of ChildrenDirective. * @param options - The options to use in configuring the child observation behavior. @@ -60,8 +62,9 @@ export class ChildrenDirective extends NodeObservationDirective< */ observe(target: any): void { const observer = - target[this.id] ?? (target[this.id] = new MutationObserver(this.handleEvent)); - observer.$fastTarget = target; + target[this.observerProperty] ?? + (target[this.observerProperty] = new MutationObserver(this.handleEvent)); + observer.target = target; observer.observe(target, this.options); } @@ -70,8 +73,8 @@ export class ChildrenDirective extends NodeObservationDirective< * @param target - The target to unobserve. */ disconnect(target: any): void { - const observer = target[this.id]; - observer.$fastTarget = null; + const observer = target[this.observerProperty]; + observer.target = null; observer.disconnect(); } @@ -88,9 +91,8 @@ export class ChildrenDirective extends NodeObservationDirective< } private handleEvent = (mutations: MutationRecord[], observer: any): void => { - const target = observer.$fastTarget; - const source = target.$fastSource; - this.updateTarget(source, this.computeNodes(target)); + const target = observer.target; + this.updateTarget(this.getSource(target), this.computeNodes(target)); }; } diff --git a/packages/web-components/fast-element/src/templating/node-observation.ts b/packages/web-components/fast-element/src/templating/node-observation.ts index 7951388f783..b78cae48f1b 100644 --- a/packages/web-components/fast-element/src/templating/node-observation.ts +++ b/packages/web-components/fast-element/src/templating/node-observation.ts @@ -51,6 +51,8 @@ export const elements = (selector?: string): ElementsFilter => export abstract class NodeObservationDirective< T extends NodeBehaviorOptions > extends StatelessAttachedAttributeDirective { + private sourceProperty = `${this.id}-s`; + /** * Bind this behavior to the source. * @param source - The source to bind to. @@ -59,7 +61,7 @@ export abstract class NodeObservationDirective< */ bind(source: any, context: ExecutionContext, targets: ViewBehaviorTargets): void { const target = targets[this.nodeId] as any; - target.$fastSource = source; + target[this.sourceProperty] = source; this.updateTarget(source, this.computeNodes(target)); this.observe(target); } @@ -74,7 +76,16 @@ export abstract class NodeObservationDirective< const target = targets[this.nodeId] as any; this.updateTarget(source, emptyArray); this.disconnect(target); - target.$fastSource = null; + target[this.sourceProperty] = null; + } + + /** + * Gets the data source for the target. + * @param target - The target to get the source for. + * @returns The source. + */ + protected getSource(target: Node) { + return target[this.sourceProperty]; } /** diff --git a/packages/web-components/fast-element/src/templating/slotted.ts b/packages/web-components/fast-element/src/templating/slotted.ts index 14436f36829..9db73ead2dd 100644 --- a/packages/web-components/fast-element/src/templating/slotted.ts +++ b/packages/web-components/fast-element/src/templating/slotted.ts @@ -45,8 +45,7 @@ export class SlottedDirective extends NodeObservationDirective