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

[labs/observers] Fix controllers not observing changes if initialized after host connected #3293

Merged
merged 7 commits into from
Sep 23, 2022
5 changes: 5 additions & 0 deletions .changeset/modern-turtles-judge.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@lit-labs/observers': patch
---

Fix controllers not observing changes to target element if initialized after the host has connected.
3 changes: 2 additions & 1 deletion packages/labs/observers/src/intersection_controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ export class IntersectionController implements ReactiveController {
host: ReactiveControllerHost,
{target, config, callback, skipInitial}: IntersectionControllerConfig
) {
(this._host = host).addController(this);
this._host = host;
// Target defaults to `host` unless explicitly `null`.
this._target =
target === null ? target : target ?? (this._host as unknown as Element);
Expand All @@ -112,6 +112,7 @@ export class IntersectionController implements ReactiveController {
},
config
);
host.addController(this);
}

/**
Expand Down
3 changes: 2 additions & 1 deletion packages/labs/observers/src/mutation_controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ export class MutationController implements ReactiveController {
host: ReactiveControllerHost,
{target, config, callback, skipInitial}: MutationControllerConfig
) {
(this._host = host).addController(this);
this._host = host;
// Target defaults to `host` unless explicitly `null`.
this._target =
target === null ? target : target ?? (this._host as unknown as Element);
Expand All @@ -104,6 +104,7 @@ export class MutationController implements ReactiveController {
this.handleChanges(records);
this._host.requestUpdate();
});
host.addController(this);
}

/**
Expand Down
3 changes: 2 additions & 1 deletion packages/labs/observers/src/performance_controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ export class PerformanceController implements ReactiveController {
host: ReactiveControllerHost,
{config, callback, skipInitial}: PerformanceControllerConfig
) {
(this._host = host).addController(this);
this._host = host;
this._config = config;
this._skipInitial = skipInitial ?? this._skipInitial;
this.callback = callback ?? this.callback;
Expand All @@ -93,6 +93,7 @@ export class PerformanceController implements ReactiveController {
this._host.requestUpdate();
}
);
host.addController(this);
}

/**
Expand Down
3 changes: 2 additions & 1 deletion packages/labs/observers/src/resize_controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ export class ResizeController implements ReactiveController {
host: ReactiveControllerHost,
{target, config, callback, skipInitial}: ResizeControllerConfig
) {
(this._host = host).addController(this);
this._host = host;
// Target defaults to `host` unless explicitly `null`.
this._target =
target === null ? target : target ?? (this._host as unknown as Element);
Expand All @@ -103,6 +103,7 @@ export class ResizeController implements ReactiveController {
this.handleChanges(entries);
this._host.requestUpdate();
});
host.addController(this);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -399,4 +399,73 @@ const canTest = () => {
await intersectionComplete();
assert.isTrue(el.observerValue);
});

test('can observe changes when initialized after host connected', async () => {
AndrewJakubowicz marked this conversation as resolved.
Show resolved Hide resolved
class TestFirstUpdated extends ReactiveElement {
observer!: IntersectionController;
observerValue: true | undefined = undefined;
override firstUpdated() {
this.observer = new IntersectionController(this, {});
}
override updated() {
this.observerValue = this.observer.value as typeof this.observerValue;
}
resetObserverValue() {
this.observer.value = this.observerValue = undefined;
}
}
customElements.define(generateElementName(), TestFirstUpdated);
const el = (await renderTestElement(TestFirstUpdated)) as TestFirstUpdated;

// Reports initial change by default
assert.isTrue(el.observerValue);

// Reports change when not intersecting
el.resetObserverValue();
intersectOut(el);
await intersectionComplete();
assert.isTrue(el.observerValue);

// Reports change when intersecting
el.resetObserverValue();
assert.isUndefined(el.observerValue);
intersectIn(el);
await intersectionComplete();
assert.isTrue(el.observerValue);
});

test('can observe external element after host connected', async () => {
const d = document.createElement('div');
container.appendChild(d);
class A extends ReactiveElement {
observer!: IntersectionController;
observerValue: true | undefined = undefined;
override firstUpdated() {
this.observer = new IntersectionController(this, {
target: d,
skipInitial: true,
});
}
override updated() {
this.observerValue = this.observer.value as typeof this.observerValue;
}
resetObserverValue() {
this.observer.value = this.observerValue = undefined;
}
}
customElements.define(generateElementName(), A);
const el = (await renderTestElement(A)) as A;

assert.equal(el.observerValue, undefined);
// Observe intersect out
intersectOut(d);
await intersectionComplete();
assert.isTrue(el.observerValue);
el.resetObserverValue();

// Observe intersect in
intersectIn(d);
await intersectionComplete();
assert.isTrue(el.observerValue);
});
});
71 changes: 71 additions & 0 deletions packages/labs/observers/src/test/mutation_controller_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -368,4 +368,75 @@ const canTest =
await nextFrame();
assert.isTrue(el.observerValue);
});

test('can observe changes when initialized after host connected', async () => {
class TestFirstUpdated extends ReactiveElement {
observer!: MutationController;
observerValue: true | undefined = undefined;
override firstUpdated() {
this.observer = new MutationController(this, {
config: {attributes: true},
});
}
override updated() {
this.observerValue = this.observer.value as typeof this.observerValue;
}
resetObserverValue() {
this.observer.value = this.observerValue = undefined;
}
}
customElements.define(generateElementName(), TestFirstUpdated);

const el = (await renderTestElement(TestFirstUpdated)) as TestFirstUpdated;

// Reports initial change by default
assert.isTrue(el.observerValue);

// Reports attribute change
el.resetObserverValue();
el.setAttribute('hi', 'hi');
await nextFrame();
assert.isTrue(el.observerValue);

// Reports another attribute change
el.resetObserverValue();
el.requestUpdate();
await nextFrame();
assert.isUndefined(el.observerValue);
el.setAttribute('bye', 'bye');
await nextFrame();
assert.isTrue(el.observerValue);
});

test('can observe external element after host connected', async () => {
class A extends ReactiveElement {
observer!: MutationController;
observerValue: true | undefined = undefined;
override firstUpdated() {
this.observer = new MutationController(this, {
target: document.body,
config: {childList: true},
skipInitial: true,
});
}
override updated() {
this.observerValue = this.observer.value as typeof this.observerValue;
}
resetObserverValue() {
this.observer.value = this.observerValue = undefined;
}
}
customElements.define(generateElementName(), A);

const el = (await renderTestElement(A)) as A;
assert.equal(el.observerValue, undefined);
const d = document.createElement('div');
document.body.appendChild(d);
await nextFrame();
assert.isTrue(el.observerValue);
el.resetObserverValue();
d.remove();
await nextFrame();
assert.isTrue(el.observerValue);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ const generateMeasure = async (sync = false) => {

const observerComplete = async (el?: HTMLElement) => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(el as any)?.observer.flush();
(el as any)?.observer?.flush();
await nextFrame();
await nextFrame();
};
Expand Down Expand Up @@ -231,4 +231,42 @@ const canTest = () => {
await observerComplete(el);
assert.match(el.observerValue as string, /2:[\d]/);
});

test('can observe changes when initialized after host connected', async () => {
class TestFirstUpdated extends ReactiveElement {
observer!: PerformanceController;
observerValue: true | undefined = undefined;
override firstUpdated() {
this.observer = new PerformanceController(this, {
config: {entryTypes: ['measure']},
});
}
override updated() {
this.observerValue = this.observer.value as typeof this.observerValue;
}
resetObserverValue() {
this.observer.value = this.observerValue = undefined;
}
}
customElements.define(generateElementName(), TestFirstUpdated);
const el = (await renderTestElement(TestFirstUpdated)) as TestFirstUpdated;

// Reports initial change by default
assert.isTrue(el.observerValue);

// Reports measure
el.resetObserverValue();
await generateMeasure();
await observerComplete(el);
assert.isTrue(el.observerValue);

// Reports another measure after noop update
el.resetObserverValue();
el.requestUpdate();
await observerComplete(el);
assert.isUndefined(el.observerValue);
await generateMeasure();
await observerComplete(el);
assert.isTrue(el.observerValue);
});
});
64 changes: 64 additions & 0 deletions packages/labs/observers/src/test/resize_controller_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -374,4 +374,68 @@ if (DEV_MODE) {
await resizeComplete();
assert.isTrue(el.observerValue);
});

test('can observe changes when initialized after host connected', async () => {
class TestFirstUpdated extends ReactiveElement {
observer!: ResizeController;
observerValue: true | undefined = undefined;
override firstUpdated() {
this.observer = new ResizeController(this, {});
}
override updated() {
this.observerValue = this.observer.value as typeof this.observerValue;
}
resetObserverValue() {
this.observer.value = this.observerValue = undefined;
}
}
customElements.define(generateElementName(), TestFirstUpdated);

const el = (await renderTestElement(TestFirstUpdated)) as TestFirstUpdated;

// Reports initial change by default
assert.isTrue(el.observerValue);

// Reports attribute change
el.resetObserverValue();
assert.isUndefined(el.observerValue);
resizeElement(el);
await resizeComplete();
assert.isTrue(el.observerValue);
});

test('can observe external element after host connected', async () => {
const d = document.createElement('div');
container.appendChild(d);
class A extends ReactiveElement {
observer!: ResizeController;
observerValue: true | undefined = undefined;
override firstUpdated() {
this.observer = new ResizeController(this, {
target: d,
skipInitial: true,
});
}
override updated() {
this.observerValue = this.observer.value as typeof this.observerValue;
}
resetObserverValue() {
this.observer.value = this.observerValue = undefined;
}
}
customElements.define(generateElementName(), A);
const el = (await renderTestElement(A)) as A;

assert.equal(el.observerValue, undefined);
resizeElement(d);
await resizeComplete();
assert.isTrue(el.observerValue);

// Change again
el.resetObserverValue();
assert.equal(el.observerValue, undefined);
resizeElement(d);
await resizeComplete();
assert.isTrue(el.observerValue);
});
});