Skip to content

Commit

Permalink
remove observableMap implementation to provide better control over to…
Browse files Browse the repository at this point in the history
…ken notification, adds a few tests
  • Loading branch information
nicholasrice committed Jun 30, 2022
1 parent 8d514e7 commit 1013266
Show file tree
Hide file tree
Showing 3 changed files with 108 additions and 121 deletions.
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { Observable, Subscriber, Updates } from "@microsoft/fast-element";
import { Observable, Subscriber } from "@microsoft/fast-element";
import { makeObservable } from "@microsoft/fast-element/utilities";
import chai, { expect } from "chai";
import spies from "chai-spies";
import { DesignTokenNode, DesignTokenResolver } from "./design-token-node.js";
import { DesignTokenNode } from "./design-token-node.js";
import { DesignToken } from "./design-token.js";

chai.use(spies);
Expand Down Expand Up @@ -572,6 +573,40 @@ describe.only("DesignTokenNode", () => {

expect(handleChange).not.to.have.been.called();
});
it("the token when the derived value assigned to a node results in the same value as the previously assigned static value", () => {
const token = new DesignToken<number>();
const node = new DesignTokenNode();
const handleChange = chai.spy(() => {})
const subscriber: Subscriber = { handleChange }
node.setTokenValue(token, 12);
Observable.getNotifier(token).subscribe(subscriber);

node.setTokenValue(token, ()=> 12);

expect(handleChange).not.to.have.been.called();
});
it("the token when the derived value assigned to a node results in the same value as the previously assigned derived value", () => {
const token = new DesignToken<number>();
const node = new DesignTokenNode();
const handleChange = chai.spy(() => {})
const subscriber: Subscriber = { handleChange }
function a() {
return 12;
}

function b() {
return 12;
}

node.setTokenValue(token, a);
Observable.getNotifier(token).subscribe(subscriber);

node.setTokenValue(token, b);

expect(a).not.to.equal(b)
expect(handleChange).not.to.have.been.called();
});

it("the token when a dependency of a derived token value is set for a descendent but there is an intermediary value set that is a static value", () => {
const token = new DesignToken<number>();
const dependency = new DesignToken<number>();
Expand Down Expand Up @@ -718,9 +753,7 @@ describe.only("DesignTokenNode", () => {
it("should get an updated value when observable properties used in a derived property are changed", () => {
const target = new DesignTokenNode();
const token = new DesignToken<number>();
const dependencies: { value: number } = {} as { value: number }
Observable.defineProperty(dependencies, "value");
dependencies.value = 6
const dependencies: { value: number } = makeObservable({ value: 6});

target.setTokenValue(token, () => dependencies.value * 2);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { BindingObserver, Observable, Subscriber } from "@microsoft/fast-element";
import { ObservableMap } from "./observable-map.js";
import { BindingObserver, Observable } from "@microsoft/fast-element";
import type { DesignToken } from "./design-token.js";

export type DesignTokenValueType =
Expand Down Expand Up @@ -91,10 +90,10 @@ export class DesignTokenNode {
private _parent: DesignTokenNode | null = null;
private _children: Set<DesignTokenNode> = new Set();
private _values: Map<DesignToken<any>, DesignTokenValue<any>> = new Map();
private _derived: ObservableMap<
private _derived: Map<
DesignToken<any>,
[DerivedDesignTokenValue<any>, StaticDesignTokenValue<any>]
> = new ObservableMap();
> = new Map();

/**
* Determines if a value is a {@link DerivedDesignTokenValue}
Expand All @@ -106,62 +105,28 @@ export class DesignTokenNode {
return typeof value === "function";
}

/**
* @returns {boolean} - true if the resulting value has changed, otherwise false.
*/
private static evaluateDerived<T>(
node: DesignTokenNode,
token: DesignToken<T>,
value: DerivedDesignTokenValue<T>
) {
): boolean {
const evaluator = DerivedValueEvaluator.getOrCreate(value);
const result = evaluator.evaluate(node, token);
const prev = node._derived.get(token);
node._derived.set(token, [value, result]);

if (prev === undefined || value !== prev[0] || result !== prev[1]) {
node._derived.set(token, [value, result]);
}

return result;
return prev === undefined ? true : prev[1] !== result;
}

/**
* Retrieves all tokens assigned directly to a node.
* @param node - the node to retrieve assigned tokens for
* @returns
*/
public static getAssignedTokensForNode(node: DesignTokenNode): DesignToken<any>[] {
return Array.from(node._values.keys());
}

/**
* Retrieves all tokens assigned to the node and ancestor nodes.
* @param node - the node to compose assigned tokens for
*/
public static composeAssignedTokensForNode(
node: DesignTokenNode
): DesignToken<any>[] {
const tokens = new Set(DesignTokenNode.getAssignedTokensForNode(node));
let current = node.parent;

while (current !== null) {
const assignedTokens = DesignTokenNode.getAssignedTokensForNode(current);

for (const token of assignedTokens) {
tokens.add(token);
}

current = current.parent;
}

return Array.from(tokens);
}

/**
* Tests if a token is assigned directly to a node
* @param node - The node to test
* @param token - The token to test
* @returns
* Notifies the token subscribes that the value has changed for the node
* @param token - The token that changed
*/
public static isAssigned<T>(node: DesignTokenNode, token: DesignToken<T>) {
return node._values.has(token);
private static notifyToken<T>(token: DesignToken<T>, node: DesignTokenNode): void {
Observable.getNotifier(token).notify(node);
}

private static isDerivedFor<T>(node: DesignTokenNode, token: DesignToken<T>) {
Expand Down Expand Up @@ -202,9 +167,46 @@ export class DesignTokenNode {

return collected;
}
/**
* Retrieves all tokens assigned directly to a node.
* @param node - the node to retrieve assigned tokens for
* @returns
*/
public static getAssignedTokensForNode(node: DesignTokenNode): DesignToken<any>[] {
return Array.from(node._values.keys());
}

/**
* Retrieves all tokens assigned to the node and ancestor nodes.
* @param node - the node to compose assigned tokens for
*/
public static composeAssignedTokensForNode(
node: DesignTokenNode
): DesignToken<any>[] {
const tokens = new Set(DesignTokenNode.getAssignedTokensForNode(node));
let current = node.parent;

while (current !== null) {
const assignedTokens = DesignTokenNode.getAssignedTokensForNode(current);

for (const token of assignedTokens) {
tokens.add(token);
}

current = current.parent;
}

constructor() {
Observable.getNotifier(this._derived).subscribe(this.derivedSubscriber);
return Array.from(tokens);
}

/**
* Tests if a token is assigned directly to a node
* @param node - The node to test
* @param token - The token to test
* @returns
*/
public static isAssigned<T>(node: DesignTokenNode, token: DesignToken<T>) {
return node._values.has(token);
}

public get parent() {
Expand Down Expand Up @@ -238,26 +240,23 @@ export class DesignTokenNode {
const derived = DesignTokenNode.collectDerivedContext(this);

if (DesignTokenNode.isDerivedTokenValue(value)) {
DesignTokenNode.evaluateDerived(this, token, value);
DesignTokenNode.evaluateDerived(this, token, value) &&
DesignTokenNode.notifyToken(token, this);

this.notifyDerived(token, DerivedValueEvaluator.getOrCreate(value), this);
} else {
// _derived.delete will notify for the token that is deleted, so only notify if deleting the value
// was unsuccessful
const notified =
DesignTokenNode.isDerivedFor(this, token) && this._derived.delete(token);
DesignTokenNode.isDerivedFor(this, token) && this._derived.delete(token);

if (prev !== value) {
if (!notified) {
Observable.getNotifier(token).notify(this);
}

DesignTokenNode.notifyToken(token, this);
this.notifyStatic(token, this);
}
}

derived.forEach((fn, token) => {
const evaluator = DerivedValueEvaluator.getOrCreate(fn);
DesignTokenNode.evaluateDerived(this, token, fn);
DesignTokenNode.evaluateDerived(this, token, fn) &&
DesignTokenNode.notifyToken(token, this);
this.notifyDerived(token, evaluator, this);
});
}
Expand Down Expand Up @@ -291,12 +290,12 @@ export class DesignTokenNode {
public deleteTokenValue<T>(token: DesignToken<T>): void {
if (DesignTokenNode.isAssigned(this, token)) {
this._values.delete(token);
const notified =
DesignTokenNode.isDerivedFor(this, token) && this._derived.delete(token);
DesignTokenNode.isDerivedFor(this, token) && this._derived.delete(token);

if (!notified) {
Observable.getNotifier(token).notify(this);
}
// TODO this should really check prev / next values
// TODO if there is an upstream value, it should notify static if the upstream is static,
// otherwise it should notify w/ the upstream derived value
DesignTokenNode.notifyToken(token, this);
}
}

Expand All @@ -313,7 +312,8 @@ export class DesignTokenNode {
const evaluator = DerivedValueEvaluator.getOrCreate(source);

if (evaluator.dependencies.has(token)) {
DesignTokenNode.evaluateDerived(this, _token, source);
DesignTokenNode.evaluateDerived(this, _token, source) &&
DesignTokenNode.notifyToken(_token, this);
this.notifyDerived(_token, evaluator, originator);
}
}
Expand All @@ -340,7 +340,8 @@ export class DesignTokenNode {
// has any dependencies of the token value. If so, we need to evaluate for this node
evaluator.dependencies.forEach(dep => {
if (DesignTokenNode.isAssigned(this, dep)) {
DesignTokenNode.evaluateDerived(this, token, evaluator.value);
DesignTokenNode.evaluateDerived(this, token, evaluator.value) &&
DesignTokenNode.notifyToken(token, this);
this.notifyDerived(token, evaluator, this);
}
});
Expand All @@ -360,7 +361,8 @@ export class DesignTokenNode {
const evaluator = DerivedValueEvaluator.getOrCreate(source);

if (evaluator.dependencies.has(token)) {
DesignTokenNode.evaluateDerived(this, _token, source);
DesignTokenNode.evaluateDerived(this, _token, source) &&
DesignTokenNode.notifyToken(_token, this);
this.notifyDerived(_token, evaluator, originator);
}
}
Expand All @@ -369,21 +371,4 @@ export class DesignTokenNode {
this.children[i].notifyDerived(token, evaluator, originator);
}
}

/**
* Change handler for derived token values
*/
private derivedSubscriber: Subscriber = {
handleChange: (source: ObservableMap, value: DesignToken<any>) => {
this.notifyToken(value);
},
};

/**
* Notifies the token subscribes that the value has changed for the node
* @param token - The token that changed
*/
private notifyToken<T>(token: DesignToken<T>): void {
Observable.getNotifier(token).notify(this);
}
}

This file was deleted.

0 comments on commit 1013266

Please sign in to comment.