fix: safer prop clone w/ define-time validation
- This implements clear errors & warnings when non-serializeable values are assigned to properties - Adjusts the size limit budget to allow better messages - Updates references to size limits
This commit is contained in:
parent
4ffb5f0674
commit
394e094693
5 changed files with 86 additions and 3 deletions
|
|
@ -36,6 +36,15 @@ There are many ways to get in touch:
|
|||
1. [fast](https://github.com/microsoft/fast) - When I found that Microsoft has their own base class I thought it was super cool!
|
||||
1. [lit](https://github.com/lit/lit) - `lit-html` continues to amaze me and I worked to make `wcb` generic so I (and others) can continue to use it
|
||||
|
||||
## Size change log
|
||||
|
||||
A running record of how each correctness/feature change affects the `WebComponent` base class bundle, measured as min + brotli via [size-limit](https://github.com/ai/size-limit). Baseline before the Phase 0 correctness work: **1.19 kB** (limit 1.2 kB).
|
||||
|
||||
| Change | Size (min+brotli) | Limit | Reason & benefit |
|
||||
| ---------------------------------------------------------------- | ------------------ | ------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
|
||||
| Move default-attribute reflection out of the constructor | ~1.19 kB (≈0) | 1.2 kB | **Custom Elements Spec compliance.** Constructors may not mutate attributes, so `document.createElement` on a class with `static props` threw `NotSupportedError` in real browsers. Defaults now reflect on first connect, and markup/SSR-provided attributes win. Pure code movement — no net size cost. |
|
||||
| Safe prop cloning (functions/instances) + define-time validation | 1.31 kB (+0.12 kB) | 1.2 → 1.35 kB | **No more `DataCloneError`.** `structuredClone` of a function/class-instance default crashed construction. Defaults are now cloned per key (plain data deep-copied so instances don't share object/array state; non-cloneable values kept by reference), and a one-time warning names any default whose type can't reflect to an attribute. Limit raised to fit. |
|
||||
|
||||
---
|
||||
|
||||
_Just keep building._<br>
|
||||
|
|
|
|||
|
|
@ -5,4 +5,4 @@ slug: library-size
|
|||
|
||||
All the functions and the base class in the library are minimalist by design and only contains what is needed for their purpose.
|
||||
|
||||
The main export (with `WebComponent` + `html`) is **1.7 kB** (min + gzip) according to [bundlephobia.com](https://bundlephobia.com/package/web-component-base@latest), and the `WebComponent` base class is **1.08 kB** (min + brotli) according to [size-limit](http://github.com/ai/size-limit).
|
||||
The main export (with `WebComponent` + `html`) is **1.7 kB** (min + gzip) according to [bundlephobia.com](https://bundlephobia.com/package/web-component-base@latest), and the `WebComponent` base class is **1.31 kB** (min + brotli) according to [size-limit](http://github.com/ai/size-limit).
|
||||
|
|
|
|||
|
|
@ -81,7 +81,7 @@
|
|||
"size-limit": [
|
||||
{
|
||||
"path": "./dist/WebComponent.js",
|
||||
"limit": "1.2 KB"
|
||||
"limit": "1.35 KB"
|
||||
},
|
||||
{
|
||||
"path": "./dist/html.js",
|
||||
|
|
|
|||
|
|
@ -11,6 +11,39 @@ import {
|
|||
deserialize,
|
||||
} from './utils/index.js'
|
||||
|
||||
/** Component classes whose defaults have been validated (once per class). */
|
||||
const validated = new WeakSet()
|
||||
|
||||
/**
|
||||
* Returns a fresh defaults object for a component: plain data is deep-copied
|
||||
* per key so instances never share object/array defaults, while non-cloneable
|
||||
* values (functions, class instances) are kept by reference instead of
|
||||
* throwing `DataCloneError`. On the first call per class it also warns (once)
|
||||
* about types that can't reflect to an attribute — those belong in
|
||||
* handlers/refs, not reflected props.
|
||||
* @param {typeof WebComponent} ctor the component constructor
|
||||
* @returns {object} a fresh defaults object
|
||||
*/
|
||||
function cloneDefaults(ctor) {
|
||||
const check = !validated.has(ctor)
|
||||
if (check) validated.add(ctor)
|
||||
const out = {}
|
||||
for (const key in ctor.props) {
|
||||
const value = ctor.props[key]
|
||||
const type = typeof value
|
||||
if (check && (type === 'function' || type === 'symbol'))
|
||||
console.warn(
|
||||
`${ctor.name}.${key}: ${type} default not reflectable; use handlers/refs.`
|
||||
)
|
||||
try {
|
||||
out[key] = structuredClone(value)
|
||||
} catch {
|
||||
out[key] = value
|
||||
}
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
/**
|
||||
* A minimal base class to reduce the complexity of creating reactive custom elements
|
||||
* @see https://webcomponent.io
|
||||
|
|
@ -158,7 +191,7 @@ export class WebComponent extends HTMLElement {
|
|||
}
|
||||
|
||||
#initializeProps() {
|
||||
let initialProps = structuredClone(this.constructor.props) ?? {}
|
||||
let initialProps = cloneDefaults(this.constructor)
|
||||
Object.keys(initialProps).forEach((camelCase) => {
|
||||
this.#typeMap[camelCase] = typeof initialProps[camelCase]
|
||||
})
|
||||
|
|
|
|||
|
|
@ -325,3 +325,44 @@ describe('default reflection (no setAttribute in constructor)', () => {
|
|||
expect(el.getAttribute('my-name')).toBe('Ayo')
|
||||
})
|
||||
})
|
||||
|
||||
/**
|
||||
* Non-cloneable defaults (functions, class instances) must not crash the
|
||||
* constructor via structuredClone's DataCloneError; they are kept by
|
||||
* reference and flagged once with a readable warning.
|
||||
*/
|
||||
describe('safe prop cloning + define-time validation', () => {
|
||||
it('constructs with a function default (kept by ref) and warns once per class', () => {
|
||||
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
|
||||
class WithFn extends WebComponent {
|
||||
static props = { fn: () => 42 }
|
||||
}
|
||||
const tag = 'clone-with-fn'
|
||||
window.customElements.define(tag, WithFn)
|
||||
|
||||
const a = document.createElement(tag)
|
||||
const b = document.createElement(tag)
|
||||
// function survives by reference and is callable on both instances
|
||||
expect(a.props.fn()).toBe(42)
|
||||
expect(b.props.fn()).toBe(42)
|
||||
// memoized per class: constructing many instances warns at most once
|
||||
expect(warn).toHaveBeenCalledOnce()
|
||||
expect(warn.mock.calls[0][0]).toContain('WithFn.fn')
|
||||
|
||||
warn.mockRestore()
|
||||
})
|
||||
|
||||
it('deep-copies object/array defaults so instances do not share them', () => {
|
||||
class WithList extends WebComponent {
|
||||
static props = { list: [1, 2] }
|
||||
}
|
||||
const tag = 'clone-with-list'
|
||||
window.customElements.define(tag, WithList)
|
||||
|
||||
const a = document.createElement(tag)
|
||||
const b = document.createElement(tag)
|
||||
a.props.list.push(3)
|
||||
expect(a.props.list).toEqual([1, 2, 3])
|
||||
expect(b.props.list).toEqual([1, 2])
|
||||
})
|
||||
})
|
||||
|
|
|
|||
Loading…
Reference in a new issue