Skip to content

Commit 2d74d0e

Browse files
theengineeartheengineear
committed
Initialize properties in constructor for React.
React doesn’t provide a way for developers to signal that they want to set a property (versus an attribute) in JSX — instead it performs some magic to _guess_ what was meant. This _guess_ is based on whether the expression `key in element` is `true`. As a best-practice, XElement is _lazy_ about initialization and waits until it is _actually connected to the DOM_ before initializing all properties. But… that means React will always check for setters _before_ they exist! This change set adds our accessor descriptors during construction, but adds an internal flag to prevent certain behavior until the component is fully initialized during connection. Closes #331.
1 parent f2fdbb1 commit 2d74d0e

File tree

8 files changed

+163
-60
lines changed

8 files changed

+163
-60
lines changed

‎test/index.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,4 +28,5 @@ test('./test-reflected-properties.html');
2828
test('./test-computed-properties.html');
2929
test('./test-observed-properties.html');
3030
test('./test-listeners.html');
31+
test('./test-property-deletion.html');
3132
test('./test-scratch.html');

‎test/test-computed-properties.js‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -229,21 +229,6 @@ it('resets compute validity on initialization to catch upgrade edge cases with i
229229
assert(el.internal.c === 3);
230230
});
231231

232-
it('throws when computed property is set before connection', () => {
233-
const el = document.createElement('test-element');
234-
el.count = undefined;
235-
let passed = false;
236-
let message = 'no error was thrown';
237-
try {
238-
el.connectedCallback();
239-
} catch (error) {
240-
const expected = 'Property "TestElement.properties.count" is computed (computed properties are read-only).';
241-
message = error.message;
242-
passed = error.message === expected;
243-
}
244-
assert(passed, message);
245-
});
246-
247232
it('cannot be written to from host', () => {
248233
const el = document.createElement('test-element');
249234
document.body.append(el);

‎test/test-property-deletion.html‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<!doctype html>
2+
<html>
3+
<head>
4+
<meta charset="utf-8">
5+
<meta http-equiv="content-security-policy" content="default-src 'self'; script-src 'self' 'sha256-RVXBMfHRdgc8777jps6ngyVOY4g0I7LMQ7IUzGv2lbA=';">
6+
<script type="importmap">
7+
{
8+
"imports": {
9+
"@netflix/x-test/": "../node_modules/@netflix/x-test/"
10+
}
11+
}
12+
</script>
13+
</head>
14+
<body>
15+
<h3>Test Property Deletion</h3>
16+
<script type="module" src="test-property-deletion.js"></script>
17+
</body>
18+
</html>

‎test/test-property-deletion.js‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
import { assert, it } from '@netflix/x-test/x-test.js';
2+
import XElement from '../x-element.js';
3+
4+
class TestElement extends XElement {
5+
static get properties() {
6+
return {
7+
foo: { type: String },
8+
bar: { type: Number },
9+
readOnlyProp: { type: String, readOnly: true },
10+
computedProp: {
11+
type: String,
12+
input: ['foo'],
13+
compute: foo => `computed-${foo}`,
14+
},
15+
};
16+
}
17+
}
18+
customElements.define('test-delete-element', TestElement);
19+
20+
it('properties are non-configurable from construction', () => {
21+
const el = document.createElement('test-delete-element');
22+
const descriptor = Object.getOwnPropertyDescriptor(el, 'foo');
23+
assert(descriptor !== undefined);
24+
assert(descriptor.configurable === false);
25+
assert(descriptor.enumerable === true);
26+
assert(typeof descriptor.get === 'function');
27+
assert(typeof descriptor.set === 'function');
28+
});
29+
30+
it('prevents deletion of properties after construction', () => {
31+
const el = document.createElement('test-delete-element');
32+
assert('foo' in el);
33+
let passed = false;
34+
let message = 'no error was thrown';
35+
try {
36+
delete el.foo;
37+
} catch (error) {
38+
message = error.message;
39+
passed = error.message.includes('Cannot delete property');
40+
}
41+
assert(passed, message);
42+
assert('foo' in el);
43+
});
44+
45+
it('returns false for Reflect.deleteProperty after construction', () => {
46+
const el = document.createElement('test-delete-element');
47+
assert('foo' in el);
48+
const result = Reflect.deleteProperty(el, 'foo');
49+
assert(result === false);
50+
assert('foo' in el);
51+
});
52+
53+
it('properties remain non-configurable after initialization', () => {
54+
const el = document.createElement('test-delete-element');
55+
document.body.append(el);
56+
const descriptor = Object.getOwnPropertyDescriptor(el, 'foo');
57+
assert(descriptor !== undefined);
58+
assert(descriptor.configurable === false);
59+
assert(descriptor.enumerable === true);
60+
assert(typeof descriptor.get === 'function');
61+
assert(typeof descriptor.set === 'function');
62+
el.remove();
63+
});

‎test/test-read-only-properties.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,3 +100,4 @@ it('cannot set to known properties', () => {
100100
}
101101
assert(passed, message);
102102
});
103+

‎ts/x-element.d.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,7 @@ export default class XElement extends HTMLElement {
161161
static "__#private@#addPropertyCompute"(constructor: any, property: any): void;
162162
static "__#private@#addPropertyObserve"(constructor: any, property: any): void;
163163
static "__#private@#constructHost"(host: any): void;
164+
static "__#private@#defineProperty"(host: any, property: any): void;
164165
static "__#private@#createInternal"(host: any): any;
165166
static "__#private@#createProperties"(host: any): any;
166167
static "__#private@#connectHost"(host: any): void;
@@ -171,7 +172,6 @@ export default class XElement extends HTMLElement {
171172
value: any;
172173
found: boolean;
173174
};
174-
static "__#private@#initializeProperty"(host: any, property: any): void;
175175
static "__#private@#addListener"(host: any, element: any, type: any, callback: any, options: any): void;
176176
static "__#private@#addListeners"(host: any): void;
177177
static "__#private@#removeListener"(host: any, element: any, type: any, callback: any, options: any): void;
@@ -181,6 +181,7 @@ export default class XElement extends HTMLElement {
181181
static "__#private@#toPathString"(host: any): string;
182182
static "__#private@#invalidateProperty"(host: any, property: any): void;
183183
static "__#private@#getPropertyValue"(host: any, property: any): any;
184+
static "__#private@#validatePropertyMutable"(host: any, property: any): void;
184185
static "__#private@#validatePropertyValue"(host: any, property: any, value: any): void;
185186
static "__#private@#setPropertyValue"(host: any, property: any, value: any): void;
186187
static "__#private@#serializeProperty"(host: any, property: any, value: any): any;

‎ts/x-element.d.ts.map‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎x-element.js‎

Lines changed: 77 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -662,10 +662,49 @@ export default class XElement extends HTMLElement {
662662
observeMap.set(property, { value: undefined });
663663
}
664664
}
665-
XElement.#hosts.set(host, {
665+
const hostInfo = {
666666
initialized: false, reflecting: false, invalidProperties, listenerMap,
667667
renderRoot, render, template, properties, internal, computeMap,
668668
observeMap, defaultMap, valueMap,
669+
};
670+
XElement.#hosts.set(host, hostInfo);
671+
XElement.#upgradeOwnProperties(host);
672+
// Define properties during construction for React 19 introspection (#331).
673+
for (const property of propertyMap.values()) {
674+
if (!property.internal) {
675+
if (Reflect.has(host, property.key)) {
676+
valueMap.set(property, host[property.key]);
677+
}
678+
XElement.#defineProperty(host, property);
679+
}
680+
}
681+
}
682+
683+
// Called during host construction.
684+
static #defineProperty(host, property) {
685+
const hostInfo = XElement.#hosts.get(host);
686+
const { valueMap } = hostInfo;
687+
const { key } = property;
688+
Reflect.defineProperty(host, key, {
689+
configurable: false,
690+
enumerable: true,
691+
get() {
692+
const { initialized } = hostInfo;
693+
if (initialized) {
694+
return XElement.#getPropertyValue(host, property);
695+
} else {
696+
return valueMap.get(property);
697+
}
698+
},
699+
set(value) {
700+
const { initialized } = hostInfo;
701+
if (initialized) {
702+
XElement.#validatePropertyMutable(host, property);
703+
XElement.#setPropertyValue(host, property, value);
704+
} else {
705+
valueMap.set(property, value);
706+
}
707+
},
669708
});
670709
}
671710

@@ -765,19 +804,21 @@ export default class XElement extends HTMLElement {
765804

766805
static #initializeHost(host) {
767806
const hostInfo = XElement.#hosts.get(host);
768-
const { computeMap, initialized, invalidProperties } = hostInfo;
807+
const { computeMap, initialized, invalidProperties, valueMap } = hostInfo;
769808
if (initialized === false) {
770-
XElement.#upgradeOwnProperties(host);
771809
// Only reflect attributes when the element is connected.
772810
const { propertyMap } = XElement.#constructors.get(host.constructor);
773811
for (const property of propertyMap.values()) {
774812
const { value, found } = XElement.#getPreUpgradePropertyValue(host, property);
775-
XElement.#initializeProperty(host, property);
776813
if (found) {
777-
host[property.key] = property.default(host, property.initial(value));
814+
XElement.#validatePropertyMutable(host, property);
815+
const initialValue = property.default(host, property.initial(value));
816+
XElement.#validatePropertyValue(host, property, initialValue);
817+
valueMap.set(property, initialValue);
778818
} else if (!property.compute) {
779-
// Set to a nullish value so that it coalesces to the default.
780-
XElement.#setPropertyValue(host, property, property.default(host, property.initial()));
819+
const initialValue = property.default(host, property.initial(value));
820+
XElement.#validatePropertyValue(host, property, initialValue);
821+
valueMap.set(property, initialValue);
781822
}
782823
invalidProperties.add(property);
783824
if (property.compute) {
@@ -804,13 +845,15 @@ export default class XElement extends HTMLElement {
804845
// Process possible sources of initial state, with this priority:
805846
// 1. imperative, e.g. `element.prop = 'value';`
806847
// 2. declarative, e.g. `<element prop="value"></element>`
807-
const { key, attribute, internal } = property;
848+
const { attribute, internal } = property;
808849
let value;
809850
let found = false;
810851
if (!internal) {
811852
// Only look for public (i.e., non-internal) properties.
812-
if (Reflect.has(host, key)) {
813-
value = host[key];
853+
const { valueMap } = XElement.#hosts.get(host);
854+
if (valueMap.has(property)) {
855+
value = valueMap.get(property);
856+
valueMap.delete(property);
814857
found = true;
815858
} else if (attribute && host.hasAttribute(attribute)) {
816859
const attributeValue = host.getAttribute(attribute);
@@ -821,25 +864,6 @@ export default class XElement extends HTMLElement {
821864
return { value, found };
822865
}
823866

824-
static #initializeProperty(host, property) {
825-
if (!property.internal) {
826-
const { key, compute, readOnly } = property;
827-
const path = `${host.constructor.name}.properties.${key}`;
828-
const get = () => XElement.#getPropertyValue(host, property);
829-
const set = compute || readOnly
830-
? () => {
831-
if (compute) {
832-
throw new Error(`Property "${path}" is computed (computed properties are read-only).`);
833-
} else {
834-
throw new Error(`Property "${path}" is read-only.`);
835-
}
836-
}
837-
: value => XElement.#setPropertyValue(host, property, value);
838-
Reflect.deleteProperty(host, key);
839-
Reflect.defineProperty(host, key, { get, set, enumerable: true });
840-
}
841-
}
842-
843867
static #addListener(host, element, type, callback, options) {
844868
callback = XElement.#getListener(host, callback);
845869
element.addEventListener(type, callback, options);
@@ -907,20 +931,18 @@ export default class XElement extends HTMLElement {
907931
}
908932

909933
static #invalidateProperty(host, property) {
910-
const { initialized, invalidProperties, computeMap } = XElement.#hosts.get(host);
911-
if (initialized) {
912-
for (const output of property.output) {
913-
XElement.#invalidateProperty(host, output);
914-
}
915-
const queueUpdate = invalidProperties.size === 0;
916-
invalidProperties.add(property);
917-
if (property.compute) {
918-
computeMap.get(property).valid = false;
919-
}
920-
if (queueUpdate) {
921-
// Batch on microtask to allow multiple, synchronous changes.
922-
queueMicrotask(() => XElement.#updateHost(host));
923-
}
934+
const { invalidProperties, computeMap } = XElement.#hosts.get(host);
935+
for (const output of property.output) {
936+
XElement.#invalidateProperty(host, output);
937+
}
938+
const queueUpdate = invalidProperties.size === 0;
939+
invalidProperties.add(property);
940+
if (property.compute) {
941+
computeMap.get(property).valid = false;
942+
}
943+
if (queueUpdate) {
944+
// Batch on microtask to allow multiple, synchronous changes.
945+
queueMicrotask(() => XElement.#updateHost(host));
924946
}
925947
}
926948

@@ -929,6 +951,18 @@ export default class XElement extends HTMLElement {
929951
return property.compute?.(host) ?? valueMap.get(property);
930952
}
931953

954+
static #validatePropertyMutable(host, property) {
955+
const { compute, readOnly, key } = property;
956+
if (compute) {
957+
const path = `${host.constructor.name}.properties.${key}`;
958+
throw new Error(`Property "${path}" is computed (computed properties are read-only).`);
959+
}
960+
if (readOnly) {
961+
const path = `${host.constructor.name}.properties.${key}`;
962+
throw new Error(`Property "${path}" is read-only.`);
963+
}
964+
}
965+
932966
static #validatePropertyValue(host, property, value) {
933967
if (property.type && XElement.#notNullish(value)) {
934968
if (XElement.#typeIsWrong(property.type, value)) {

0 commit comments

Comments
 (0)