Skip to content

Commit

Permalink
Revert "updates base class to set firstRender to false immediately to…
Browse files Browse the repository at this point in the history
… prevent double mounts"

This reverts commit dd0ff86.
  • Loading branch information
George Treviranus committed May 16, 2021
1 parent 3ebb155 commit 33d2623
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 41 deletions.
2 changes: 1 addition & 1 deletion src/__tests__/fixtures/accessor-fixture.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { UpgradedElement, register } from "../.."
* can't unregister custom elements from the DOM, even between tests.
* @param {string} id
*/
export function createAccessorFixture(id) {
export function accessorFixture(id) {
class TestElement extends UpgradedElement {
constructor() {
super()
Expand Down
2 changes: 1 addition & 1 deletion src/__tests__/fixtures/basic-fixture.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { UpgradedElement, register } from "../.."
* @param {string} id
* @param {{content: string, properties: Object, styles: string}}
*/
export function createBasicFixture(id, options = {}) {
export function basicFixture(id, options = {}) {
class TestElement extends UpgradedElement {
constructor() {
super()
Expand Down
2 changes: 1 addition & 1 deletion src/__tests__/fixtures/lifecycle-fixture.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { UpgradedElement, register } from "../.."
* can't unregister custom elements from the DOM, even between tests.
* @param {string} id
*/
export function createLifecycleFixture(id, wait = false) {
export function lifecycleFixture(id, wait = false) {
class TestElement extends UpgradedElement {
constructor() {
super()
Expand Down
66 changes: 38 additions & 28 deletions src/__tests__/upgraded-element.spec.js
Original file line number Diff line number Diff line change
@@ -1,32 +1,33 @@
import { createBasicFixture } from "./fixtures/basic-fixture"
import { createAccessorFixture } from "./fixtures/accessor-fixture"
import { createLifecycleFixture } from "./fixtures/lifecycle-fixture"
import { basicFixture } from "./fixtures/basic-fixture"
import { accessorFixture } from "./fixtures/accessor-fixture"
import { lifecycleFixture } from "./fixtures/lifecycle-fixture"
import { getElement } from "./fixtures/get-element"
import * as external from "../external"

window.requestAnimationFrame = jest.fn().mockImplementation((fn) => fn())
window.cancelAnimationFrame = jest.fn().mockImplementation((fn) => fn())

describe("UpgradedElement", () => {
afterEach(() => (document.innerHTML = ""))

it("upgrades the element", () => {
// Given
createBasicFixture("upgraded")
basicFixture("upgraded")
// Then
expect(getElement("upgraded").hasAttribute("element-id")).toBe(true)
})

it("creates a shadow root", () => {
// Given
createBasicFixture("creates-shadow")
basicFixture("creates-shadow")
// Then
expect(getElement("creates-shadow").shadowRoot).not.toBeNull()
})

it("renders styles to shadow root", () => {
// Given
const styles = "div { display: block; }"
createBasicFixture("styles", { styles })
basicFixture("styles", { styles })
// Then
expect(
getElement("styles").shadowRoot.querySelector("style")
Expand All @@ -39,7 +40,7 @@ describe("UpgradedElement", () => {
it("assigns slots, if given", () => {
// Given
const slotName = "main"
createBasicFixture("slotted", {
basicFixture("slotted", {
slotName,
content: `<slot name='main'></slot>`,
})
Expand All @@ -55,7 +56,7 @@ describe("UpgradedElement", () => {
const properties = {
testProp1: { default: "foo" },
}
createBasicFixture("props", { properties })
basicFixture("props", { properties })
// Then
expect(getElement("props").testProp1).toEqual(
properties.testProp1.default
Expand All @@ -68,7 +69,7 @@ describe("UpgradedElement", () => {
testProp1: { default: "foo" },
}
const nextValue = "bar"
createBasicFixture("val-change", { properties })
basicFixture("val-change", { properties })
// When
getElement("val-change").testProp1 = nextValue
// Then
Expand All @@ -80,7 +81,7 @@ describe("UpgradedElement", () => {

it("doesn't upgrade properties if accessors already exist", () => {
// Given
createAccessorFixture("no-upgrade")
accessorFixture("no-upgrade")
// Then
expect(getElement("no-upgrade").count).toEqual(1)
})
Expand All @@ -95,7 +96,7 @@ describe("UpgradedElement", () => {
safe: true,
},
}
createBasicFixture("safe-upgrade", { properties })
basicFixture("safe-upgrade", { properties })
// Then
const nextValue = "&lt;span&gt;unsafe&lt;/span&gt;"
expect(getElement("safe-upgrade").safeString).toEqual(nextValue)
Expand All @@ -110,7 +111,7 @@ describe("UpgradedElement", () => {
safe: true,
},
}
createBasicFixture("safe-change", { properties })
basicFixture("safe-change", { properties })
// When
getElement("safe-change").safeString = "&hello"
// Then
Expand All @@ -125,7 +126,7 @@ describe("UpgradedElement", () => {
const properties = {
reflectedProp: { reflected: true },
}
createBasicFixture("reflect-one", { properties })
basicFixture("reflect-one", { properties })
// Then
const element = getElement("reflect-one")
expect(element.hasAttribute("reflected-prop")).toBe(true)
Expand All @@ -137,7 +138,7 @@ describe("UpgradedElement", () => {
const properties = {
reflectedProp: { default: "foo", reflected: true },
}
createBasicFixture("reflect-two", { properties })
basicFixture("reflect-two", { properties })
// Then
const element = getElement("reflect-two")
expect(element.hasAttribute("reflected-prop")).toBe(true)
Expand All @@ -149,7 +150,7 @@ describe("UpgradedElement", () => {
const properties = {
reflectedProp: { default: "foo", reflected: true },
}
createBasicFixture("reflect-three", { properties })
basicFixture("reflect-three", { properties })
const element = getElement("reflect-three")
element.reflectedProp = "bar"
// Then
Expand All @@ -161,7 +162,7 @@ describe("UpgradedElement", () => {
const properties = {
reflectedProp: { default: "foo", reflected: true },
}
createBasicFixture("reflect-four", { properties })
basicFixture("reflect-four", { properties })
const element = getElement("reflect-four")
element.reflectedProp = undefined
// Then
Expand All @@ -184,7 +185,7 @@ describe("UpgradedElement", () => {
default: "foo",
},
}
createBasicFixture("upgrade-type-warn", { properties })
basicFixture("upgrade-type-warn", { properties })
// Then
expect(console.warn).toBeCalledWith(warningMessage)
})
Expand All @@ -197,7 +198,7 @@ describe("UpgradedElement", () => {
default: true,
},
}
createBasicFixture("change-type-warn", { properties })
basicFixture("change-type-warn", { properties })
// When
getElement("change-type-warn").testProp1 = "foo"
// Then
Expand All @@ -211,7 +212,7 @@ describe("UpgradedElement", () => {

describe("lifecycle methods", () => {
it("calls elementPropertyChanged", () => {
const Cls = createLifecycleFixture("prop-changed")
const Cls = lifecycleFixture("prop-changed")
Cls.prototype[external.elementPropertyChanged] = jest.fn()
getElement("prop-changed").testProp = true
expect(Cls.prototype[external.elementPropertyChanged]).toBeCalledWith(
Expand All @@ -222,7 +223,7 @@ describe("UpgradedElement", () => {
})

it("calls elementAttributeChanged", () => {
const Cls = createLifecycleFixture("attr-changed")
const Cls = lifecycleFixture("attr-changed")
Cls.prototype[external.elementAttributeChanged] = jest.fn()
getElement("attr-changed").testProp = true
expect(Cls.prototype[external.elementAttributeChanged]).toBeCalledWith(
Expand All @@ -233,45 +234,54 @@ describe("UpgradedElement", () => {
})

it("calls elementDidUpdate", () => {
const Cls = createLifecycleFixture("update")
const Cls = lifecycleFixture("update")
Cls.prototype[external.elementDidUpdate] = jest.fn()
getElement("update").testProp = true
expect(Cls.prototype[external.elementDidUpdate]).toBeCalled()
})

it("calls elementDidUpdate if property updates in elementDidMount", async () => {
const [init, Cls] = createLifecycleFixture("mount-update", true)
// whyyyyy
// eslint-disable-next-line jest/no-disabled-tests
it.skip("calls elementDidUpdate if property updates in elementDidMount", async () => {
const [init, Cls] = lifecycleFixture("mount-update", true)
Cls.prototype[external.elementDidUpdate] = jest.fn()
Cls.prototype[external.elementDidMount] = () => {
getElement("mount-update").testProp = true
}
Cls.prototype[external.elementDidUpdate] = jest.fn()
// Cls.prototype[external.elementDidMount] = () => {
// console.log("Mounted!")
// getElement("mount-update").testProp = true
// }
// Cls.prototype[external.elementDidUpdate] = () => {
// console.log("Updated!")
// }
init()
expect(Cls.prototype[external.elementDidUpdate]).toBeCalled()
})

it("calls elementDidConnect", () => {
const [init, Cls] = createLifecycleFixture("connect", true)
const [init, Cls] = lifecycleFixture("connect", true)
Cls.prototype[external.elementDidConnect] = jest.fn()
init()
expect(Cls.prototype[external.elementDidConnect]).toBeCalled()
})

it("calls elementDidMount", () => {
const [init, Cls] = createLifecycleFixture("mount", true)
const [init, Cls] = lifecycleFixture("mount", true)
Cls.prototype[external.elementDidMount] = jest.fn()
init()
expect(Cls.prototype[external.elementDidMount]).toBeCalled()
})

it("calls elementWillUnmount", () => {
const Cls = createLifecycleFixture("unmount")
const Cls = lifecycleFixture("unmount")
Cls.prototype[external.elementWillUnmount] = jest.fn()
document.body.removeChild(getElement("unmount"))
expect(Cls.prototype[external.elementWillUnmount]).toBeCalled()
})

it("recalls elementDidMount if the component is disconnected and then reconnected", async () => {
const [init, Cls] = createLifecycleFixture("remount", true)
const [init, Cls] = lifecycleFixture("remount", true)
Cls.prototype[external.elementDidMount] = jest.fn()
init()
const fixture = getElement("remount")
Expand Down
4 changes: 2 additions & 2 deletions src/internal.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,6 @@ export const runLifecycle = Symbol("#runLifecycle")
export const performUpgrade = Symbol("#performUpgrade")
export const upgradeProperty = Symbol("#upgradeProperty")
export const renderStyles = Symbol("#renderStyles")
export const setInitialRenderState = Symbol("#setInitialRenderState")
export const setNextRenderState = Symbol("#setNextRenderState")
export const getInitialRenderState = Symbol("#getInitialRenderState")
export const getNextRenderState = Symbol("#getNextRenderState")
export const renderDOM = Symbol("#renderDOM")
14 changes: 6 additions & 8 deletions src/upgraded-element.js
Original file line number Diff line number Diff line change
Expand Up @@ -220,17 +220,18 @@ export class UpgradedElement extends HTMLElement {
* Create a virtual DOM from the external `render` method and patch
* it into the shadow root. Triggers `elementDidMount`, if defined.
*/
[internal.setInitialRenderState]() {
[internal.getInitialRenderState]() {
this[internal.vDOM] = this[internal.getVDOM]()
render(this[internal.vDOM], this[internal.shadowRoot])
this[internal.runLifecycle](external.elementDidMount)
this[internal.isFirstRender] = false
}

/**
* All renders after initial render:
* Create a new vdom and patch the existing one.
*/
[internal.setNextRenderState]() {
[internal.getNextRenderState]() {
let nextVDOM = this[internal.getVDOM]()
patch(nextVDOM, this[internal.vDOM])
this[internal.runLifecycle](external.elementDidUpdate)
Expand All @@ -241,11 +242,8 @@ export class UpgradedElement extends HTMLElement {
* Runs either a new render or diffs the existing virtual DOM to a new one.
*/
[internal.renderDOM]() {
if (this[internal.isFirstRender]) {
this[internal.isFirstRender] = false
this[internal.setInitialRenderState]()
} else {
this[internal.setNextRenderState]()
}
return this[internal.isFirstRender]
? this[internal.getInitialRenderState]()
: this[internal.getNextRenderState]()
}
}

0 comments on commit 33d2623

Please sign in to comment.