Skip to content
This repository has been archived by the owner on Apr 4, 2022. It is now read-only.

Remove MutationObserver, overload setAttribute instead #34

Merged
merged 13 commits into from
Jan 29, 2021
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
14 changes: 12 additions & 2 deletions examples/attribute-mutation-performance.html
Original file line number Diff line number Diff line change
Expand Up @@ -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! -->
Expand All @@ -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>
Expand Down
7 changes: 6 additions & 1 deletion examples/reusing-resources.html
Original file line number Diff line number Diff line change
Expand Up @@ -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)">
Expand Down
27 changes: 10 additions & 17 deletions src/BaseElement.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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 }))

Expand Down Expand Up @@ -217,10 +214,6 @@ export class BaseElement extends HTMLElement {

/* Invoke removedCallback */
this.removedCallback()

/* Disconnect observer */
this._observer?.disconnect()
this._observer = undefined
})
}
}
Expand Down
5 changes: 1 addition & 4 deletions test/args.test.ts
Original file line number Diff line number Diff line change
@@ -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"

Expand All @@ -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")
})
})
18 changes: 1 addition & 17 deletions test/three-element.test.ts
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -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")
})

Expand All @@ -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)
})

Expand All @@ -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)
})

Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down