From 05ff0861646c7b37f210b42499234d182df03909 Mon Sep 17 00:00:00 2001 From: George Treviranus Date: Thu, 3 Feb 2022 19:28:44 -0800 Subject: [PATCH] feat: allows attribute values to be property defaults if a property is reflected but has no default value --- src/__tests__/fixtures/basic-fixture.js | 8 ++- src/__tests__/rotom.spec.js | 14 +++++- src/properties/initialize-property-value.js | 50 +++++++++---------- src/properties/validate-type.js | 6 +-- test/jsx/fixtures/kitchen-sink-test.js | 2 + test/jsx/fixtures/nested-element-test.js | 6 ++- test/jsx/fixtures/render-schedule-test.js | 4 +- test/jsx/index.html | 8 ++- test/template/fixtures/kitchen-sink-test.js | 4 ++ test/template/fixtures/nested-element-test.js | 2 +- test/template/index.html | 8 ++- 11 files changed, 74 insertions(+), 38 deletions(-) diff --git a/src/__tests__/fixtures/basic-fixture.js b/src/__tests__/fixtures/basic-fixture.js index 4cfbecd..ce0b7ea 100644 --- a/src/__tests__/fixtures/basic-fixture.js +++ b/src/__tests__/fixtures/basic-fixture.js @@ -44,7 +44,13 @@ export function createBasicFixture(id, options = {}) { register(`test-${id}`, TestElement) if (options.content && options.content.includes("slot")) { - document.body.innerHTML = `
Test slot
` + document.body.innerHTML = ` + +
Test slot
+ + ` + } else if (options.attribute) { + document.body.innerHTML = `` } else { document.body.innerHTML = `` } diff --git a/src/__tests__/rotom.spec.js b/src/__tests__/rotom.spec.js index 154daf9..0096446 100644 --- a/src/__tests__/rotom.spec.js +++ b/src/__tests__/rotom.spec.js @@ -193,7 +193,19 @@ describe("RotomElement", () => { expect(element.hasAttribute("reflected-prop")).toBe(false) }) - describe("property is undefined", () => { + it("uses attribute value for undefined property default", () => { + const properties = { + reflectedProp: { reflected: true }, + } + createBasicFixture("reflect-no-prop-default", { + properties, + attribute: "reflected-prop='foo'", + }) + const element = getElement("reflect-no-prop-default") + expect(element.reflectedProp).toEqual("foo") + }) + + describe("property set to undefined", () => { let element beforeEach(() => { diff --git a/src/properties/initialize-property-value.js b/src/properties/initialize-property-value.js index 8e936ee..48b0db1 100644 --- a/src/properties/initialize-property-value.js +++ b/src/properties/initialize-property-value.js @@ -11,34 +11,25 @@ import { export const initializePropertyValue = ( Cls, propName, - configuration, + { default: defaultValue, type: propType, reflected = false, safe = false }, privateName ) => { - const { - default: defaultValue, - type: propType, - reflected = false, - safe = false, - } = configuration + // If the property happens to be pre-existing, set aside + // the old value to a separate prop and use its value on + // the replacement - // Initializing the property value: - // + if (!isUndefined(Cls[propName])) { + Cls[getTempName(propName)] = Cls[propName] + } + + // Initialize the value // 1. If the default is a function, compute the value - // 2. If the prop name happens to be an existing property, set aside - // the property's value to a separate prop and use its value on - // the replacement - // 3. Otherwise, just set the value as the default + // 2. Otherwise, just set the value as the default let initialValue if (isFunction(defaultValue)) { initialValue = defaultValue(Cls) - } else if (!isUndefined(Cls[propName])) { - initialValue = Cls[propName] - - // Set aside the initial value to a new property, before it's - // deleted by the new accessors. - Cls[getTempName(propName)] = initialValue } else { initialValue = defaultValue } @@ -46,7 +37,11 @@ export const initializePropertyValue = ( // Validate the property's default value type, if given // Initialize the private property - if (!isUndefined(initialValue)) { + const valueEmpty = isUndefined(initialValue) + const attrName = camelToKebab(propName) + const attrValue = Cls.getAttribute(attrName) + + if (!valueEmpty) { if (BUILD_ENV === "development") { validateType(Cls, propName, initialValue, propType) } @@ -56,13 +51,18 @@ export const initializePropertyValue = ( } Cls[privateName] = initialValue - } + } else if (reflected && attrValue) { + // if reflected, attribute value exists, and no default given, + // set the attribute value to the private prop + // + // attributes can only have strings, so no need to type check - // If the value is reflected, set its attribute. + initialValue = safe ? sanitizeString(attrValue) : attrValue + Cls[privateName] = initialValue + } - if (reflected) { + if (reflected && !attrValue) { const initialAttrValue = initialValue ? String(initialValue) : "" - const attribute = camelToKebab(propName) - Cls.setAttribute(attribute, initialAttrValue) + Cls.setAttribute(attrName, initialAttrValue) } } diff --git a/src/properties/validate-type.js b/src/properties/validate-type.js index b3caf74..039ed74 100644 --- a/src/properties/validate-type.js +++ b/src/properties/validate-type.js @@ -1,4 +1,4 @@ -import { log, getTypeTag } from "../utilities" +import { log, getTypeTag, isUndefined } from "../utilities" /** * Checks that a prop name matches its intended type. @@ -8,11 +8,11 @@ import { log, getTypeTag } from "../utilities" * @param {string} type */ export function validateType(Cls, propName, value, type) { - if (typeof type === "undefined") return + if (isUndefined(type)) return const evaluatedType = getTypeTag(value) - if (type === undefined || evaluatedType === type) return + if (evaluatedType === type) return log( `Property '${propName}' is invalid type of '${evaluatedType}'. Expected '${type}'. Check ${Cls.constructor.name}.` diff --git a/test/jsx/fixtures/kitchen-sink-test.js b/test/jsx/fixtures/kitchen-sink-test.js index 8543869..0204bc4 100644 --- a/test/jsx/fixtures/kitchen-sink-test.js +++ b/test/jsx/fixtures/kitchen-sink-test.js @@ -28,6 +28,7 @@ export class KitchenSinkTest extends RotomElement { default: "", safe: true, }, + attributeDefault: { reflected: true }, } } @@ -111,6 +112,7 @@ export class KitchenSinkTest extends RotomElement {

{this.getAttribute("description")}

{renderRemovedNote}

Sanitized content: {this.safeString}

+

Attribute set to prop: {this.attributeDefault}

diff --git a/test/jsx/fixtures/nested-element-test.js b/test/jsx/fixtures/nested-element-test.js index 24ed4a6..d8951a2 100644 --- a/test/jsx/fixtures/nested-element-test.js +++ b/test/jsx/fixtures/nested-element-test.js @@ -69,7 +69,11 @@ class NestedElementTest extends RotomElement { borderColor: this.borderColor, }} > - + diff --git a/test/jsx/fixtures/render-schedule-test.js b/test/jsx/fixtures/render-schedule-test.js index 8bf0fe4..9d91646 100644 --- a/test/jsx/fixtures/render-schedule-test.js +++ b/test/jsx/fixtures/render-schedule-test.js @@ -40,7 +40,7 @@ export class RenderScheduleTest extends RotomElement { render() { return (

Render count: {this.count}

-

+

This is a scheduling test.
It should have one render per button press, despite having multiple diff --git a/test/jsx/index.html b/test/jsx/index.html index 231d465..3e4bf5d 100644 --- a/test/jsx/index.html +++ b/test/jsx/index.html @@ -16,10 +16,14 @@

Render schedule test (should start at 1)

Kitchen sink

- +

Lifecycles (see console)

- +

(this button will remove the component, cause a disconnect/unmount, then on append, re-call connect/mount)

diff --git a/test/template/fixtures/kitchen-sink-test.js b/test/template/fixtures/kitchen-sink-test.js index d4d7b6f..5c6d798 100644 --- a/test/template/fixtures/kitchen-sink-test.js +++ b/test/template/fixtures/kitchen-sink-test.js @@ -26,6 +26,7 @@ export class KitchenSinkTest extends RotomElement { default: "", safe: true, }, + attributeDefault: { reflected: true }, } } @@ -125,6 +126,9 @@ export class KitchenSinkTest extends RotomElement {

${this.getAttribute("description")}

${intermittentNode}

Sanitized content: ${this.safeString}

+

Attribute set to prop: ${ + this.attributeDefault + }

diff --git a/test/template/fixtures/nested-element-test.js b/test/template/fixtures/nested-element-test.js index 5b60655..e6095e4 100644 --- a/test/template/fixtures/nested-element-test.js +++ b/test/template/fixtures/nested-element-test.js @@ -70,7 +70,7 @@ class NestedElementTest extends RotomElement {

This one is nested with inline styles.

- +
diff --git a/test/template/index.html b/test/template/index.html index 414ffec..6c665fd 100644 --- a/test/template/index.html +++ b/test/template/index.html @@ -16,10 +16,14 @@

Render schedule test (should start at 1)

Kitchen sink

- +

Lifecycles (see console)

- +

(this button will remove the component, cause a disconnect/unmount, then on append, re-call connect/mount)