Skip to content

Commit

Permalink
fix: increase sophistication of behavior binding and unbinding (micro…
Browse files Browse the repository at this point in the history 
…soft#4288)

* track number of binds and unbind

* adding force-remove logic and moving isConnected assignemnt back to its original location

* removing accidently added import

* update api-report

* switch to for/of from itterator

Co-authored-by: nicholasrice <nicholasrice@users.noreply.github.com>
Co-authored-by: Ibrahim Maga <imaga75@hotmail.com>
  • Loading branch information
@nicholasrice @Ibrahimmaga
3 people authored Jan 30, 2021
1 parent df6f765 commit 9c24ee6
Show file tree
Hide file tree
Showing 3 changed files with 167 additions and 30 deletions.
2 changes: 1 addition & 1 deletion packages/web-components/fast-element/docs/api-report.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ export class Controller extends PropertyChangeNotifier {
onAttributeChangedCallback(name: string, oldValue: string, newValue: string): void;
onConnectedCallback(): void;
onDisconnectedCallback(): void;
removeBehaviors(behaviors: ReadonlyArray<Behavior>): void;
removeBehaviors(behaviors: ReadonlyArray<Behavior>, force?: boolean): void;
removeStyles(styles: ElementStyles | HTMLStyleElement): void;
get styles(): ElementStyles | null;
set styles(value: ElementStyles | null);
Expand Down
150 changes: 137 additions & 13 deletions packages/web-components/fast-element/src/controller.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { html } from "./template";
import { DOM } from "./dom";
import { css } from "./styles";
import { Observable } from "./observation/observable";
import { Behavior } from "./directives/behavior";

describe("The Controller", () => {
const templateA = html`a`;
Expand Down Expand Up @@ -320,23 +321,146 @@ describe("The Controller", () => {
expect(attached).to.equal(false);
});

it("should attach and detach the HTMLStyleElement supplied to .addStyles() and .removeStyles() to the shadowRoot", () => {
const { controller, element } = createController({
shadowOptions: {
mode: "open",
},
template: templateA,
it("should attach and detach the HTMLStyleElement supplied to .addStyles() and .removeStyles() to the shadowRoot", () => {
const { controller, element } = createController({
shadowOptions: {
mode: "open",
},
template: templateA,
});

const style = document.createElement("style") as HTMLStyleElement;
expect(element.shadowRoot?.contains(style)).to.equal(false);

controller.addStyles(style);

expect(element.shadowRoot?.contains(style)).to.equal(true);

controller.removeStyles(style);

expect(element.shadowRoot?.contains(style)).to.equal(false);
});
it("should attach and detach the HTMLStyleElement supplied to .addStyles() and .removeStyles() to the shadowRoot", () => {
const { controller, element } = createController({
shadowOptions: {
mode: "open",
},
template: templateA,
});

const style = document.createElement("style") as HTMLStyleElement;
expect(element.shadowRoot?.contains(style)).to.equal(false);
const style = document.createElement("style") as HTMLStyleElement;
expect(element.shadowRoot?.contains(style)).to.equal(false);

controller.addStyles(style);
controller.addStyles(style);

expect(element.shadowRoot?.contains(style)).to.equal(true);
expect(element.shadowRoot?.contains(style)).to.equal(true);

controller.removeStyles(style);
controller.removeStyles(style);

expect(element.shadowRoot?.contains(style)).to.equal(false);
});
expect(element.shadowRoot?.contains(style)).to.equal(false);
});

context("with behaviors", () => {
it("should bind all behaviors added prior to connection, during connection", () => {
class TestBehavior implements Behavior {
public bound = false;
bind() {
this.bound = true;
}
unbind() {
this.bound = false;
}
}

const behaviors = [new TestBehavior(), new TestBehavior(), new TestBehavior()];
const { controller, element } = createController();
controller.addBehaviors(behaviors);

behaviors.forEach(x => expect(x.bound).to.equal(false))

document.body.appendChild(element);

behaviors.forEach(x => expect(x.bound).to.equal(true));
});

it("should bind a behavior B that is added to the Controller by behavior A, where A is added prior to connection and B is added during A's bind()", () => {
let childBehaviorBound = false;
class ParentBehavior implements Behavior {
bind(el: FASTElement) {
el.$fastController.addBehaviors([new ChildBehavior()])
}

unbind() {}
}

class ChildBehavior implements Behavior {
bind(el: FASTElement) {
childBehaviorBound = true;
}

unbind() {}
}



const { element, controller } = createController();
controller.addBehaviors([new ParentBehavior()]);
document.body.appendChild(element);

expect(childBehaviorBound).to.equal(true);
});
it("should unbind a behavior only when the behavior is removed the number of times it has been added", () => {
class TestBehavior implements Behavior {
public bound = false;
bind() {
this.bound = true;
}

unbind() {
this.bound = false;
}
}

const behavior = new TestBehavior();
const { element, controller } = createController();

document.body.appendChild(element);

controller.addBehaviors([behavior]);
controller.addBehaviors([behavior]);
controller.addBehaviors([behavior]);

expect(behavior.bound).to.equal(true);
controller.removeBehaviors([behavior]);
expect(behavior.bound).to.equal(true);
controller.removeBehaviors([behavior]);
expect(behavior.bound).to.equal(true);
controller.removeBehaviors([behavior]);
expect(behavior.bound).to.equal(false);
});
it("should unbind a behavior whenever the behavior is removed with the force argument", () => {
class TestBehavior implements Behavior {
public bound = false;
bind() {
this.bound = true;
}

unbind() {
this.bound = false;
}
}

const behavior = new TestBehavior();
const { element, controller } = createController();

document.body.appendChild(element);

controller.addBehaviors([behavior]);
controller.addBehaviors([behavior]);

expect(behavior.bound).to.equal(true);
controller.removeBehaviors([behavior], true);
expect(behavior.bound).to.equal(false);
});
})
});
45 changes: 29 additions & 16 deletions packages/web-components/fast-element/src/controller.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ function getShadowRoot(element: HTMLElement): ShadowRoot | null {
*/
export class Controller extends PropertyChangeNotifier {
private boundObservables: Record<string, any> | null = null;
private behaviors: Behavior[] | null = null;
private behaviors: Map<Behavior, number> | null = null;
private needsInitialization = true;
private _template: ElementViewTemplate | null = null;
private _styles: ElementStyles | null = null;
Expand Down Expand Up @@ -195,48 +195,62 @@ export class Controller extends PropertyChangeNotifier {
* @param behaviors - The behaviors to add.
*/
public addBehaviors(behaviors: ReadonlyArray<Behavior>): void {
const targetBehaviors = this.behaviors || (this.behaviors = []);
const targetBehaviors = this.behaviors || (this.behaviors = new Map());
const length = behaviors.length;
const behaviorsToBind: Behavior[] = [];

for (let i = 0; i < length; ++i) {
targetBehaviors.push(behaviors[i]);
const behavior = behaviors[i];

if (targetBehaviors.has(behavior)) {
targetBehaviors.set(behavior, targetBehaviors.get(behavior) + 1);
} else {
targetBehaviors.set(behavior, 1);
behaviorsToBind.push(behavior);
}
}

if (this.isConnected) {
const element = this.element;

for (let i = 0; i < length; ++i) {
behaviors[i].bind(element, defaultExecutionContext);
for (let i = 0; i < behaviorsToBind.length; ++i) {
behaviorsToBind[i].bind(element, defaultExecutionContext);
}
}
}

/**
* Removes behaviors from this element.
* @param behaviors - The behaviors to remove.
* @param force - Forces unbinding of behaviors.
*/
public removeBehaviors(behaviors: ReadonlyArray<Behavior>): void {
public removeBehaviors(behaviors: ReadonlyArray<Behavior>, force = false): void {
const targetBehaviors = this.behaviors;

if (targetBehaviors === null) {
return;
}

const length = behaviors.length;
const behaviorsToUnbind: Behavior[] = [];

for (let i = 0; i < length; ++i) {
const index = targetBehaviors.indexOf(behaviors[i]);
const behavior = behaviors[i];

if (index !== -1) {
targetBehaviors.splice(index, 1);
if (targetBehaviors.has(behavior)) {
const count = targetBehaviors.get(behavior)! - 1;

count === 0 || force
? targetBehaviors.delete(behavior) && behaviorsToUnbind.push(behavior)
: targetBehaviors.set(behavior, count);
}
}

if (this.isConnected) {
const element = this.element;

for (let i = 0; i < length; ++i) {
behaviors[i].unbind(element);
for (let i = 0; i < behaviorsToUnbind.length; ++i) {
behaviorsToUnbind[i].unbind(element);
}
}
}
Expand All @@ -260,8 +274,8 @@ export class Controller extends PropertyChangeNotifier {
const behaviors = this.behaviors;

if (behaviors !== null) {
for (let i = 0, ii = behaviors.length; i < ii; ++i) {
behaviors[i].bind(element, defaultExecutionContext);
for (let [behavior] of behaviors) {
behavior.bind(element, defaultExecutionContext);
}
}

Expand All @@ -288,9 +302,8 @@ export class Controller extends PropertyChangeNotifier {

if (behaviors !== null) {
const element = this.element;

for (let i = 0, ii = behaviors.length; i < ii; ++i) {
behaviors[i].unbind(element);
for (let [behavior] of behaviors) {
behavior.unbind(element);
}
}
}
Expand Down

0 comments on commit 9c24ee6

Please sign in to comment.