Skip to content

Commit

Permalink
Merge 38535fc into 87b8172
Browse files Browse the repository at this point in the history
  • Loading branch information
EisenbergEffect authored Oct 31, 2022
2 parents 87b8172 + 38535fc commit be8beea
Show file tree
Hide file tree
Showing 18 changed files with 294 additions and 42 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "fix: prevent circular reference errors when stringifying DOM nodes controlled by FAST's rendering engine",
"packageName": "@microsoft/fast-element",
"email": "roeisenb@microsoft.com",
"dependentChangeType": "prerelease"
}
8 changes: 8 additions & 0 deletions packages/web-components/fast-element/docs/api-report.md
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,8 @@ export class ElementController<TElement extends HTMLElement = HTMLElement> exten
readonly source: TElement;
get template(): ElementViewTemplate<TElement> | null;
set template(value: ElementViewTemplate<TElement> | null);
// @internal
toJSON: () => undefined;
readonly view: ElementView<TElement> | null;
}

Expand Down Expand Up @@ -518,6 +520,8 @@ export class HTMLView<TSource = any, TParent = any> implements ElementView<TSour
readonly sourceLifetime: SourceLifetime;
// (undocumented)
readonly targets: ViewBehaviorTargets;
// @internal
toJSON: () => undefined;
unbind(): void;
}

Expand Down Expand Up @@ -758,6 +762,8 @@ export abstract class StatelessAttachedAttributeDirective<TOptions> implements H
nodeId: string;
// (undocumented)
protected options: TOptions;
// @internal
toJSON: () => undefined;
}

// @public
Expand Down Expand Up @@ -908,6 +914,8 @@ export class ViewTemplate<TSource = any, TParent = any> implements ElementViewTe
readonly factories: Record<string, ViewBehaviorFactory>;
readonly html: string | HTMLTemplateElement;
render(source: TSource, host: Node, hostBindingTarget?: Element): HTMLView<TSource, TParent>;
// @internal
toJSON: () => undefined;
}

// @public
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -634,4 +634,12 @@ describe("The ElementController", () => {
expect(order[6]).to.equal('parent behavior disconnected');
expect(order[7]).to.equal('child behavior disconnected');
});

it("should not throw if DOM stringified", () => {
const controller = createController();

expect(() => {
JSON.stringify(controller.element);
}).to.not.throw();
});
});
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Message, Mutable, StyleStrategy, StyleTarget } from "../interfaces.js";
import { Message, Mutable, noop, StyleStrategy, StyleTarget } from "../interfaces.js";
import { PropertyChangeNotifier } from "../observation/notifier.js";
import { Observable, SourceLifetime } from "../observation/observable.js";
import { FAST } from "../platform.js";
Expand Down Expand Up @@ -438,6 +438,12 @@ export class ElementController<TElement extends HTMLElement = HTMLElement>
return false;
}

/**
* Opts out of JSON stringification.
* @internal
*/
toJSON = noop;

private renderTemplate(template: ElementViewTemplate | null | undefined): void {
// When getting the host to render to, we start by looking
// up the shadow root. If there isn't one, then that means
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
isString,
KernelServiceId,
Message,
noop,
} from "../interfaces.js";
import { createMetadataLocator, FAST } from "../platform.js";
import { Updates } from "./update-queue.js";
Expand Down Expand Up @@ -254,6 +255,11 @@ export const Observable = FAST.getById(KernelServiceId.observable, () => {
super(expression, initialSubscriber);
}

/**
* Opts out of JSON stringification.
*/
toJSON = noop;

public setMode(isAsync: boolean): void {
this.isAsync = this.needsQueue = isAsync;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import type {
ExpressionController,
ExpressionObserver,
} from "../observation/observable.js";
import { isString } from "../interfaces.js";
import { isString, noop } from "../interfaces.js";
import type { Subscriber } from "../observation/notifier.js";
import type { HTMLBindingDirective } from "./binding.js";
import { Binding } from "./html-directive.js";
Expand Down Expand Up @@ -78,6 +78,12 @@ class SignalObserver<TSource = any, TReturn = any, TParent = any> implements Sub
this.subscriber.handleChange(this.dataBinding.evaluate, this);
}

/**
* Opts out of JSON stringification.
* @internal
*/
toJSON = noop;

private getSignal(controller: ExpressionController<TSource, TParent>): string {
const options = this.dataBinding.options;
return isString(options)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { isString, Message } from "../interfaces.js";
import { isString, Message, noop } from "../interfaces.js";
import type { Subscriber } from "../observation/notifier.js";
import {
ExecutionContext,
Expand Down Expand Up @@ -133,6 +133,12 @@ class TwoWayObserver<TSource = any, TReturn = any, TParent = any>
value
);
}

/**
* Opts out of JSON stringification.
* @internal
*/
toJSON = noop;
}

class TwoWayBinding<TSource = any, TReturn = any, TParent = any> extends Binding<
Expand Down
105 changes: 104 additions & 1 deletion packages/web-components/fast-element/src/templating/binding.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { html, ViewTemplate } from "./template.js";
import { toHTML } from "../__test__/helpers.js";
import { SyntheticView, HTMLView } from "./view.js";
import { Updates } from "../observation/update-queue.js";
import { Aspect, HTMLDirective } from "./html-directive.js";
import { Aspect } from "./html-directive.js";
import { DOM } from "./dom.js";
import { Signal, signal } from "./binding-signal.js";
import { twoWay, TwoWayBindingOptions } from "./binding-two-way.js";
Expand Down Expand Up @@ -120,6 +120,20 @@ describe("The HTML binding directive", () => {

expect(node.textContent).to.equal(model.value);
});

it("should not throw if DOM stringified", () => {
const { behavior, node, targets } = contentBinding();
const model = new Model("This is a test");
const controller = Fake.viewController(targets, behavior);

controller.bind(model);

expect(node.textContent).to.equal(model.value);

expect(() => {
JSON.stringify(node);
}).to.not.throw();
});
});

context("when binding template content", () => {
Expand Down Expand Up @@ -284,6 +298,20 @@ describe("The HTML binding directive", () => {

expect(toHTML(parentNode)).to.equal(`<a>Hi there!</a>`);
});

it("target node should not stringify $fastView or $fastTemplate", () => {
const { behavior, node, targets } = contentBinding();
const template = html<Model>`This is a template. ${x => x.knownValue}`;
const model = new Model(template);
const controller = Fake.viewController(targets, behavior);

controller.bind(model);

const clone = JSON.parse(JSON.stringify(node));

expect("$fastView" in clone).to.be.false;
expect("$fastTemplate" in clone).to.be.false;
});
})

context("when unbinding template content", () => {
Expand Down Expand Up @@ -423,6 +451,18 @@ describe("The HTML binding directive", () => {

expect(aspectScenario.getValue(node)).to.equal(aspectScenario.originalValue);
});

it("should not throw if DOM stringified", () => {
const { behavior, node, targets } = defaultBinding(aspectScenario.sourceAspect);
const model = new Model(aspectScenario.originalValue);
const controller = Fake.viewController(targets, behavior);

controller.bind(model);

expect(() => {
JSON.stringify(node);
}).to.not.throw();
});
}
});

Expand Down Expand Up @@ -469,6 +509,18 @@ describe("The HTML binding directive", () => {

expect(aspectScenario.getValue(node)).to.equal(aspectScenario.originalValue);
});

it("should not throw if DOM stringified", () => {
const { behavior, node, targets } = oneTimeBinding(aspectScenario.sourceAspect);
const model = new Model(aspectScenario.originalValue);
const controller = Fake.viewController(targets, behavior);

controller.bind(model);

expect(() => {
JSON.stringify(node);
}).to.not.throw();
});
}
});

Expand Down Expand Up @@ -525,6 +577,18 @@ describe("The HTML binding directive", () => {

expect(aspectScenario.getValue(node)).to.equal(aspectScenario.originalValue);
});

it("should not throw if DOM stringified", () => {
const { behavior, node, targets } = signalBinding("test-signal", aspectScenario.sourceAspect);
const model = new Model(aspectScenario.originalValue);
const controller = Fake.viewController(targets, behavior);

controller.bind(model);

expect(() => {
JSON.stringify(node);
}).to.not.throw();
});
}
});

Expand Down Expand Up @@ -624,6 +688,18 @@ describe("The HTML binding directive", () => {

expect(aspectScenario.getValue(node)).to.equal(aspectScenario.originalValue);
});

it("should not throw if DOM stringified", () => {
const { behavior, node, targets } = twoWayBinding({}, aspectScenario.sourceAspect);
const model = new Model(aspectScenario.originalValue);
const controller = Fake.viewController(targets, behavior);

controller.bind(model);

expect(() => {
JSON.stringify(node);
}).to.not.throw();
});
}
});

Expand Down Expand Up @@ -686,6 +762,18 @@ describe("The HTML binding directive", () => {
node.dispatchEvent(new CustomEvent("my-event"));
expect(model.actionInvokeCount).to.equal(1);
});

it("should not throw if DOM stringified", () => {
const { behavior, targets, node } = eventBinding({}, "@my-event");
const model = new Model("Test value.");
const controller = Fake.viewController(targets, behavior);

controller.bind(model);

expect(() => {
JSON.stringify(node);
}).to.not.throw();
});
});

context('when binding classList', () => {
Expand Down Expand Up @@ -749,5 +837,20 @@ describe("The HTML binding directive", () => {
updateTarget(element, observerA, undefined);
expect(contains('foo')).false;
});

it("should not throw if DOM stringified", () => {
const directive = new HTMLBindingDirective(bind(() => ""));
const { behavior, node, targets } = configureDirective(directive, ":classList");

Aspect.assign(directive, ":classList");
const model = new Model("Test value.");
const controller = Fake.viewController(targets, behavior);

controller.bind(model);

expect(() => {
JSON.stringify(node);
}).to.not.throw();
});
});
});
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { Subscriber } from "../index.js";
import { isFunction, Message } from "../interfaces.js";
import { isFunction, Message, noop } from "../interfaces.js";
import {
ExecutionContext,
Expression,
Expand Down Expand Up @@ -57,6 +57,12 @@ class OneTimeBinding<TSource = any, TReturn = any, TParent = any>
bind(controller: ExpressionController): TReturn {
return this.evaluate(controller.source, controller.context);
}

/**
* Opts out of JSON stringification.
* @internal
*/
toJSON = noop;
}

type UpdateTarget = (
Expand Down
Loading

0 comments on commit be8beea

Please sign in to comment.