Remove MutationObserver, overload setAttribute instead (#34) (closes #19

)
hmans · Jan 29, 2021 · e74ca07 · e74ca07
1 parent 11854ad
commit e74ca07
Show file tree

Hide file tree

Showing 6 changed files with 32 additions and 41 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -74,6 +74,8 @@ If you've been extending ThreeElement in your own code, or hacking on the codeba
 
 - **Breaking Change:** Ticker events are now emitted by the three-game's `emitter`. Since we're no longer using DOM events, this means we also no longer need the `ticking` property/attribute, so it has been removed.
 
+- **Changed:** Instead of using a MutationObserver instance to monitor the element for updated attributes (we can't feasibly make use of `observedAttributes`, remember?), we now simply hook into `setAttribute` to react on attribute changes. This yields _significant_ (order of a magnitude) performance improvements in projects that go through element attributes a lot. Sadly, it also means that changes you're making to the DOM in your browser's dev tools will no longer be picked up automatically (but this feature may make a comeback at a later date.)
+
 - **Holy crap:** `applyProps` was refactored to use `if` instead of `switch (true)`. All you Senior JavaScript Architects can finally calm down, for I am no longer impeding upon your creed!
 
 - **Changed:** `yarn dev` now makes use of the excellent [@web/dev-server](https://modern-web.dev/docs/dev-server/overview/). This allows us to get rid of the importmap shim we had been using so far, load additional dependencies straight from our own `node_modules`, and greatly increase iteration speed during development.

diff --git a/examples/attribute-mutation-performance.html b/examples/attribute-mutation-performance.html
@@ -8,6 +8,16 @@
     />
   </head>
   <body>
+    <!--
+
+      IMPORTANT NOTE!
+
+      The following code is doing some things that are definitely NOT considered best
+      practices. It solely exists for benchmarking different approaches for tracking
+      attribute changes to elements.
+
+    -->
+
     <div id="root"></div>
 
     <!-- Import dependencies via ESM. The future is now! -->
@@ -29,14 +39,14 @@
                 color="#f30"
               ></three-mesh-standard-material>
 
-              <three-fog near="0" far="64" color="#111"></three-fog>
+              <three-fog near="0" far="32" color="#111"></three-fog>
               <three-ambient-light intensity="0.2"></three-ambient-light>
               <three-directional-light
                 position="10, 10, 40"
                 intensity="0.8"
               ></three-directional-light>
 
-              ${Swarm(100)}
+              ${Swarm(1000)}
 
               <three-orbit-controls></three-orbit-controls>
             </three-scene>

diff --git a/examples/reusing-resources.html b/examples/reusing-resources.html
@@ -16,7 +16,12 @@
 
         <!-- resources we'll reuse -->
         <three-box-buffer-geometry id="geometry" args="1, 5, 1"></three-box-buffer-geometry>
-        <three-mesh-standard-material id="material" color="#555"></three-mesh-standard-material>
+        <three-mesh-standard-material
+          id="material"
+          color="gold"
+          metalness="0.5"
+          roughness="0.5"
+        ></three-mesh-standard-material>
 
         <!-- Scene Contents -->
         <three-group ontick="this.object.rotateX(0.001); this.object.rotateY(0.002)">

diff --git a/src/BaseElement.ts b/src/BaseElement.ts
@@ -2,7 +2,6 @@ import * as THREE from "three"
 import { ThreeGame, TickerFunction } from "./elements/three-game"
 import { ThreeScene } from "./elements/three-scene"
 import { IConstructable } from "./types"
-import { observeAttributeChange } from "./util/observeAttributeChange"
 
 /**
  * The `BaseElement` class extends the built-in HTMLElement class with a bit of convenience
@@ -132,8 +131,16 @@ export class BaseElement extends HTMLElement {
     this.requestFrame = this.requestFrame.bind(this)
   }
 
-  /** This element's MutationObserver. */
-  private _observer?: MutationObserver
+  /**
+   * We're overloading setAttribute so it also invokes attributeChangedCallback. We
+   * do this because we can't realistically make use of observedAttributes (since we don't
+   * know at the time element classes are defined what properties their wrapped objects
+   * are exposing.)
+   */
+  setAttribute(name: string, value: string) {
+    this.attributeChangedCallback(name, this.getAttribute(name)!, value)
+    super.setAttribute(name, value)
+  }
 
   /**
    * This callback is invoked when the element is deemed properly initialized. Most
@@ -152,16 +159,6 @@ export class BaseElement extends HTMLElement {
   connectedCallback() {
     this.debug("connectedCallback")
 
-    /*
-    When one of this element's attributes changes, apply it to the object. Custom Elements have a built-in
-    mechanism for this (attributeChangedCallback and observedAttributes, but unfortunately we can't use it,
-    since we don't know the set of attributes the wrapped Three.js classes expose beforehand. So instead
-    we're hacking our way around it using a mutation observer. Fun times!)
-    */
-    this._observer ||= observeAttributeChange(this, (prop, value) => {
-      this.attributeChangedCallback(prop, (this as any)[prop], value)
-    })
-
     /* Emit connected event */
     this.dispatchEvent(new CustomEvent("connected", { bubbles: true, cancelable: false }))
 
@@ -217,10 +214,6 @@ export class BaseElement extends HTMLElement {
 
         /* Invoke removedCallback */
         this.removedCallback()
-
-        /* Disconnect observer */
-        this._observer?.disconnect()
-        this._observer = undefined
       })
     }
   }

diff --git a/test/args.test.ts b/test/args.test.ts
@@ -1,4 +1,4 @@
-import { expect, fixture, html, nextFrame } from "@open-wc/testing"
+import { expect, fixture, html } from "@open-wc/testing"
 import "../src"
 import { ThreeElement } from "../src/ThreeElement"
 
@@ -17,9 +17,6 @@ describe("the args attribute", () => {
   it("provides the arguments for the Three.js constructor", async () => {
     const game = await render()
     const fog = game.querySelector("three-fog") as ThreeElement
-
-    await nextFrame()
-
     expect(fog.object.color.getHexString()).to.equal("333333")
   })
 })
diff --git a/test/three-element.test.ts b/test/three-element.test.ts
@@ -1,4 +1,4 @@
-import { expect, html, nextFrame } from "@open-wc/testing"
+import { expect, html } from "@open-wc/testing"
 import * as THREE from "three"
 import "../src"
 import { ThreeElement } from "../src/ThreeElement"
@@ -26,12 +26,9 @@ describe("<three-*> powered by ThreeElement", () => {
   describe("assigning to an attribute", () => {
     it("sets the wrapped object's property of the same name", async () => {
       const el = await renderMeshElement()
-
       expect(el.object.name).to.equal("")
 
       el.setAttribute("name", "A good mesh")
-      await nextFrame()
-
       expect(el.object.name).to.equal("A good mesh")
     })
 
@@ -41,8 +38,6 @@ describe("<three-*> powered by ThreeElement", () => {
       expect(el.object.position.x).to.equal(0)
 
       el.setAttribute("position.x", "1")
-      await nextFrame()
-
       expect(el.object.position.x).to.equal(1)
     })
 
@@ -53,8 +48,6 @@ describe("<three-*> powered by ThreeElement", () => {
       expect(el.object.position.x).to.equal(0)
 
       el.setAttribute("position:x", "1")
-      await nextFrame()
-
       expect(el.object.position.x).to.equal(1)
     })
 
@@ -67,8 +60,6 @@ describe("<three-*> powered by ThreeElement", () => {
       expect(el.object.position.z).to.equal(0)
 
       el.setAttribute("position", "1, 2, 3")
-      await nextFrame()
-
       expect(el.object.position.x).to.equal(1)
       expect(el.object.position.y).to.equal(2)
       expect(el.object.position.z).to.equal(3)
@@ -84,8 +75,6 @@ describe("<three-*> powered by ThreeElement", () => {
       expect(el.object.scale.z).to.equal(0)
 
       el.setAttribute("scale", "1")
-      await nextFrame()
-
       expect(el.object.scale.x).to.equal(1)
       expect(el.object.scale.y).to.equal(1)
       expect(el.object.scale.z).to.equal(1)
@@ -96,27 +85,22 @@ describe("<three-*> powered by ThreeElement", () => {
         const el = await renderMeshElement()
 
         el.setAttribute("rotation.x", "90deg")
-        await nextFrame()
         expect(el.object.rotation.x).to.equal(Math.PI / 2)
 
         el.setAttribute("rotation.x", "-90deg")
-        await nextFrame()
         expect(el.object.rotation.x).to.equal(Math.PI / -2)
 
         el.setAttribute("rotation.x", "0deg")
-        await nextFrame()
         expect(el.object.rotation.x).to.equal(0)
 
         el.setAttribute("rotation.x", "-0.5deg")
-        await nextFrame()
         expect(el.object.rotation.x).to.equal((Math.PI / 180) * -0.5)
       })
 
       it("works within lists", async () => {
         const el = await renderMeshElement()
 
         el.setAttribute("rotation", "90deg, 0, -90deg")
-        await nextFrame()
         expect(el.object.rotation.x).to.equal(Math.PI / 2)
         expect(el.object.rotation.y).to.equal(0)
         expect(el.object.rotation.z).to.equal(Math.PI / -2)