Skip to content

Commit

Permalink
feat: allows attribute values to be property defaults if a property i…
Browse files Browse the repository at this point in the history
…s reflected but has no default value
  • Loading branch information
geotrev committed Feb 4, 2022
1 parent ab9ed86 commit 05ff086
Show file tree
Hide file tree
Showing 11 changed files with 74 additions and 38 deletions.
8 changes: 7 additions & 1 deletion src/__tests__/fixtures/basic-fixture.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,13 @@ export function createBasicFixture(id, options = {}) {
register(`test-${id}`, TestElement)

if (options.content && options.content.includes("slot")) {
document.body.innerHTML = `<test-${id}><div slot='${options.slotName}'>Test slot</div></test-${id}>`
document.body.innerHTML = `
<test-${id}>
<div slot='${options.slotName}'>Test slot</div>
</test-${id}>
`
} else if (options.attribute) {
document.body.innerHTML = `<test-${id} ${options.attribute}></test-${id}>`
} else {
document.body.innerHTML = `<test-${id}></test-${id}>`
}
Expand Down
14 changes: 13 additions & 1 deletion src/__tests__/rotom.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down
50 changes: 25 additions & 25 deletions src/properties/initialize-property-value.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,42 +11,37 @@ 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
}

// 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)
}
Expand All @@ -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)
}
}
6 changes: 3 additions & 3 deletions src/properties/validate-type.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { log, getTypeTag } from "../utilities"
import { log, getTypeTag, isUndefined } from "../utilities"

/**
* Checks that a prop name matches its intended type.
Expand All @@ -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}.`
Expand Down
2 changes: 2 additions & 0 deletions test/jsx/fixtures/kitchen-sink-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export class KitchenSinkTest extends RotomElement {
default: "<script>;(function(){console.log('sanitized')})()</script>",
safe: true,
},
attributeDefault: { reflected: true },
}
}

Expand Down Expand Up @@ -111,6 +112,7 @@ export class KitchenSinkTest extends RotomElement {
<p key="desc">{this.getAttribute("description")}</p>
{renderRemovedNote}
<p key="safe">Sanitized content: {this.safeString}</p>
<p key="attr-default">Attribute set to prop: {this.attributeDefault}</p>
<button key="name-btn" on-click={this.handleNameChange}>
Change Name
</button>
Expand Down
6 changes: 5 additions & 1 deletion test/jsx/fixtures/nested-element-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,11 @@ class NestedElementTest extends RotomElement {
borderColor: this.borderColor,
}}
>
<kitchen-sink-test first-name={this.name} description="I'm nested!">
<kitchen-sink-test
attribute-default="it works!"
first-name={this.name}
description="I'm nested!"
>
<slot></slot>
</kitchen-sink-test>
</div>
Expand Down
4 changes: 2 additions & 2 deletions test/jsx/fixtures/render-schedule-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,15 @@ export class RenderScheduleTest extends RotomElement {
render() {
return (
<div
class-name="container"
className="container"
style={{
borderWidth: "4px",
borderStyle: "solid",
borderColor: this.borderColor,
}}
>
<p>Render count: {this.count}</p>
<p class-name="label" style={{ color: this.labelColor }}>
<p className="label" style={{ color: this.labelColor }}>
This is a scheduling test.
<br />
It should have one render per button press, despite having multiple
Expand Down
8 changes: 6 additions & 2 deletions test/jsx/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,14 @@ <h2>Render schedule test (should start at 1)</h2>
<render-schedule-test></render-schedule-test>
<hr />
<h2>Kitchen sink</h2>
<kitchen-sink-test first-name="Silver" description="This is my favorite group of people."></kitchen-sink-test>
<kitchen-sink-test
attribute-default="it works!"
first-name="Silver"
description="This is my favorite group of people."
></kitchen-sink-test>
<hr />
<h2>Lifecycles (see console)</h2>
<lifecycle-test first-name="Amy" description="Testing lifecycles..."></lifecycle-test>
<lifecycle-test attribute-default="it works!" first-name="Amy" description="Testing lifecycles..."></lifecycle-test>
<br />
<button class="change-btn">Remove above element</button>
<p>(this button will remove the component, cause a disconnect/unmount, then on append, re-call connect/mount)</p>
Expand Down
4 changes: 4 additions & 0 deletions test/template/fixtures/kitchen-sink-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export class KitchenSinkTest extends RotomElement {
default: "<script>;(function(){console.log('sanitized')})()</script>",
safe: true,
},
attributeDefault: { reflected: true },
}
}

Expand Down Expand Up @@ -125,6 +126,9 @@ export class KitchenSinkTest extends RotomElement {
<p data-key='desc'>${this.getAttribute("description")}</p>
${intermittentNode}
<p data-key="safe">Sanitized content: ${this.safeString}</p>
<p key="attr-default">Attribute set to prop: ${
this.attributeDefault
}</p>
<button data-key="name-btn" id="update-name-btn">Change Name</button>
<button data-key="hl-btn" id="update-hl-btn">Remove Highlights</button>
<button data-key="attr-btn" id="update-attr-btn">Remove Attribute</button>
Expand Down
2 changes: 1 addition & 1 deletion test/template/fixtures/nested-element-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ class NestedElementTest extends RotomElement {
<p data-key="lede">This one is nested with inline styles.</p>
<button data-key="updater" id="clicker">Click to update</button>
<div data-key="nested" class="border" style="border-color: ${this.borderColor}">
<kitchen-sink-test first-name="${this.name}" description="I'm nested!">
<kitchen-sink-test attribute-default="it works!" first-name="${this.name}" description="I'm nested!">
<slot></slot>
</kitchen-sink-test>
</div>
Expand Down
8 changes: 6 additions & 2 deletions test/template/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,14 @@ <h2>Render schedule test (should start at 1)</h2>
<render-schedule-test></render-schedule-test>
<hr />
<h2>Kitchen sink</h2>
<kitchen-sink-test first-name="Silver" description="This is my favorite group of people."></kitchen-sink-test>
<kitchen-sink-test
attribute-default="it works!"
first-name="Silver"
description="This is my favorite group of people."
></kitchen-sink-test>
<hr />
<h2>Lifecycles (see console)</h2>
<lifecycle-test first-name="Amy" description="Testing lifecycles..."></lifecycle-test>
<lifecycle-test attribute-default="it works!" first-name="Amy" description="Testing lifecycles..."></lifecycle-test>
<br />
<button class="change-btn">Remove above element</button>
<p>(this button will remove the component, cause a disconnect/unmount, then on append, re-call connect/mount)</p>
Expand Down

0 comments on commit 05ff086

Please sign in to comment.