Merge b0ca645 into d8aead0

lit · Sep 23, 2022 · 1a5662a · 1a5662a
2 parents d8aead0 + b0ca645
commit 1a5662a
Show file tree

Hide file tree

Showing 9 changed files with 256 additions and 5 deletions.
diff --git a/.changeset/modern-turtles-judge.md b/.changeset/modern-turtles-judge.md
@@ -0,0 +1,5 @@
+---
+'@lit-labs/observers': patch
+---
+
+Fix controllers not observing changes to target element if initialized after the host has connected.
diff --git a/packages/labs/observers/src/intersection_controller.ts b/packages/labs/observers/src/intersection_controller.ts
@@ -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);
@@ -112,6 +112,7 @@ export class IntersectionController implements ReactiveController {
       },
       config
     );
+    host.addController(this);
   }
 
   /**

diff --git a/packages/labs/observers/src/mutation_controller.ts b/packages/labs/observers/src/mutation_controller.ts
@@ -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);
@@ -104,6 +104,7 @@ export class MutationController implements ReactiveController {
       this.handleChanges(records);
       this._host.requestUpdate();
     });
+    host.addController(this);
   }
 
   /**

diff --git a/packages/labs/observers/src/performance_controller.ts b/packages/labs/observers/src/performance_controller.ts
@@ -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;
@@ -93,6 +93,7 @@ export class PerformanceController implements ReactiveController {
         this._host.requestUpdate();
       }
     );
+    host.addController(this);
   }
 
   /**

diff --git a/packages/labs/observers/src/resize_controller.ts b/packages/labs/observers/src/resize_controller.ts
@@ -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);
@@ -103,6 +103,7 @@ export class ResizeController implements ReactiveController {
       this.handleChanges(entries);
       this._host.requestUpdate();
     });
+    host.addController(this);
   }
 
   /**

diff --git a/packages/labs/observers/src/test/intersection_controller_test.ts b/packages/labs/observers/src/test/intersection_controller_test.ts
@@ -399,4 +399,73 @@ const canTest = () => {
     await intersectionComplete();
     assert.isTrue(el.observerValue);
   });
+
+  test('can observe changes when initialized after host connected', async () => {
+    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);
+  });
 });
diff --git a/packages/labs/observers/src/test/mutation_controller_test.ts b/packages/labs/observers/src/test/mutation_controller_test.ts
@@ -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);
+  });
 });
diff --git a/packages/labs/observers/src/test/performance_controller_test.ts b/packages/labs/observers/src/test/performance_controller_test.ts
@@ -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();
 };
@@ -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);
+  });
 });
diff --git a/packages/labs/observers/src/test/resize_controller_test.ts b/packages/labs/observers/src/test/resize_controller_test.ts
@@ -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);
+  });
 });