Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: enable stringification of DOM nodes controlled by FAST's renderer #6485

Merged
merged 9 commits into from
Oct 31, 2022
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 @@ -576,4 +576,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 @@ -417,6 +417,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
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