Skip to content

Commit

Permalink
Differentiate null and undefined in Custom Elements - removing sets t…
Browse files Browse the repository at this point in the history
…o undefined (#28716)

In React DOM, in general, we don't differentiate between `null` and
`undefined` because we expect to target DOM APIs. When we're setting a
property on a Custom Element, in the new heuristic, the goal is to allow
passing whatever data type instead of normalizing it. Switching between
`undefined` and `null` as an explicit value should therefore be
respected.

However, in this mode if `undefined` is used for the initial value, we
don't actually set the property at all. If passing `null` we will now
initialize it to the value `null`. Meaning `undefined` kind of
represents the default.

### Removing Properties

There is a pretty complex edge case which is what should happen when a
prop used to exist but was removed from the props object. This doesn't
have any kind of defined semantics. It really should mean - return to
"default". Because in the declarative world it means the same as if it
was just created - i.e. we can't just leave it as it was.

The closest might be `delete object.property` but that's not really the
intended way that properties on custom elements / classes are supposed
to operate. Additionally, for a property to even hit our heuristic it
must pass the `in` test and must exist to being with so the default must
have a value.

Since the point of these properties is to contain any kind of type,
there isn't really a conceptual default value. E.g. a numeric default
value might be zero `0` while a default string might be empty `""` and
default object might `null`. Additionally, the conceptual default can
really be initialized to anything. There's also varied precedence in the
ecosystem here and really no consensus. Anything we pick would be kind
of wrong, so we used to just pick `null`.

_The safest way to consume a Custom Element is to always pass the same
set of props._

JS does have a concept of a "default value" though and that is described
as the value `undefined`. That's why default argument / object property
initializers are initialized if the value is `undefined`.

The problem with using `undefined` as value is that [you shouldn't
actually ever set the value of a class property to
`undefined`](https://twitter.com/sebmarkbage/status/1774082540296388752).
A property should always be initialized to some value. It can't be left
missing and shouldn't be initialized to the value `undefined` for hidden
class optimizations. If we just mutate it to be `undefined` it would be
potentially bad for perf and shouldn't really be the value after
removing property - it should be returned to default.

Every property should really have a setter to be useful since it is what
is used to trigger reactivity when it changes. Sometimes you can just
use the properties passively when something else happens but most of the
time it should be a setter but to reach parity with DOM it should really
be always so that the active value can be normalized.

Those setters can have default argument initializers to represent what
the default value should be. Therefore Custom Element properties should
be used like this:

```js
class CustomElement extends HTMLElement {
  _textLabel = '';
  _price = 0;
  _items = null;

  constructor() {
    super();
  }
  set textLabel(value = '') {
    this._textLabel = value;
  }
  get textLabel() {
    return this._textLabel;
  }
  set price(value = 0) {
    this._price = value;
  }
  get price() {
    return this._price;
  }
  set items(value = null) {
    this._items = value;
  }
  get items() {
    return this._items;
  }
}
```

The default initializer can be used to initialize a value back to its
original default when `undefined` is passed to it. Therefore, we pass
`undefined`, not because we expect that to be the value of a property
but because that's the value that represents "return to default".

This fixes #28203 but not really for the reason specified in the issue.
We don't expect you to actually store the `undefined` value but to use a
setter to set the property to something else that represents the
default. When we initialize the element the first time, we won't set
anything if it's the value `undefined` so we assume that the property
initializers running in the constructor is going to set the same default
value as if we set the property to `undefined`.

cc @josepharhar

DiffTrain build for [48ec17b](48ec17b)
  • Loading branch information
sebmarkbage committed Apr 2, 2024
1 parent 9350ce4 commit 385c181
Show file tree
Hide file tree
Showing 11 changed files with 73 additions and 73 deletions.
2 changes: 1 addition & 1 deletion compiled/facebook-www/REVISION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
ba5496d411a2d3067b5aae58b882aa2041807e77
48ec17b865f439754fcdaa289ef0aa98f15a05c2
12 changes: 6 additions & 6 deletions compiled/facebook-www/ReactDOM-dev.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -36279,7 +36279,7 @@ if (__DEV__) {
return root;
}

var ReactVersion = "19.0.0-www-classic-7bc0e095";
var ReactVersion = "19.0.0-www-classic-fd4e175d";

function createPortal$1(
children,
Expand Down Expand Up @@ -42151,7 +42151,7 @@ if (__DEV__) {

var _propValue5 = props[_propKey5];

if (_propValue5 == null) {
if (_propValue5 === undefined) {
continue;
}

Expand All @@ -42161,7 +42161,7 @@ if (__DEV__) {
_propKey5,
_propValue5,
props,
null
undefined
);
}

Expand Down Expand Up @@ -42695,14 +42695,14 @@ if (__DEV__) {

if (
lastProps.hasOwnProperty(_propKey16) &&
_lastProp10 != null &&
_lastProp10 !== undefined &&
!nextProps.hasOwnProperty(_propKey16)
) {
setPropOnCustomElement(
domElement,
tag,
_propKey16,
null,
undefined,
nextProps,
_lastProp10
);
Expand All @@ -42716,7 +42716,7 @@ if (__DEV__) {
if (
nextProps.hasOwnProperty(_propKey17) &&
_nextProp5 !== _lastProp11 &&
(_nextProp5 != null || _lastProp11 != null)
(_nextProp5 !== undefined || _lastProp11 !== undefined)
) {
setPropOnCustomElement(
domElement,
Expand Down
12 changes: 6 additions & 6 deletions compiled/facebook-www/ReactDOM-dev.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -36127,7 +36127,7 @@ if (__DEV__) {
return root;
}

var ReactVersion = "19.0.0-www-modern-f1b0fe47";
var ReactVersion = "19.0.0-www-modern-0973d70b";

function createPortal$1(
children,
Expand Down Expand Up @@ -42874,7 +42874,7 @@ if (__DEV__) {

var _propValue5 = props[_propKey5];

if (_propValue5 == null) {
if (_propValue5 === undefined) {
continue;
}

Expand All @@ -42884,7 +42884,7 @@ if (__DEV__) {
_propKey5,
_propValue5,
props,
null
undefined
);
}

Expand Down Expand Up @@ -43418,14 +43418,14 @@ if (__DEV__) {

if (
lastProps.hasOwnProperty(_propKey16) &&
_lastProp10 != null &&
_lastProp10 !== undefined &&
!nextProps.hasOwnProperty(_propKey16)
) {
setPropOnCustomElement(
domElement,
tag,
_propKey16,
null,
undefined,
nextProps,
_lastProp10
);
Expand All @@ -43439,7 +43439,7 @@ if (__DEV__) {
if (
nextProps.hasOwnProperty(_propKey17) &&
_nextProp5 !== _lastProp11 &&
(_nextProp5 != null || _lastProp11 != null)
(_nextProp5 !== undefined || _lastProp11 !== undefined)
) {
setPropOnCustomElement(
domElement,
Expand Down
16 changes: 8 additions & 8 deletions compiled/facebook-www/ReactDOM-prod.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -14845,14 +14845,14 @@ function setInitialProperties(domElement, tag, props) {
for (defaultChecked in props)
props.hasOwnProperty(defaultChecked) &&
((propKey = props[defaultChecked]),
null != propKey &&
void 0 !== propKey &&
setPropOnCustomElement(
domElement,
tag,
defaultChecked,
propKey,
props,
null
void 0
));
return;
}
Expand Down Expand Up @@ -15150,13 +15150,13 @@ function updateProperties(domElement, tag, lastProps, nextProps) {
for (var propKey$244 in lastProps)
(propKey$218 = lastProps[propKey$244]),
lastProps.hasOwnProperty(propKey$244) &&
null != propKey$218 &&
void 0 !== propKey$218 &&
!nextProps.hasOwnProperty(propKey$244) &&
setPropOnCustomElement(
domElement,
tag,
propKey$244,
null,
void 0,
nextProps,
propKey$218
);
Expand All @@ -15165,7 +15165,7 @@ function updateProperties(domElement, tag, lastProps, nextProps) {
(propKey = lastProps[defaultChecked]),
!nextProps.hasOwnProperty(defaultChecked) ||
propKey$218 === propKey ||
(null == propKey$218 && null == propKey) ||
(void 0 === propKey$218 && void 0 === propKey) ||
setPropOnCustomElement(
domElement,
tag,
Expand Down Expand Up @@ -17070,7 +17070,7 @@ Internals.Events = [
var devToolsConfig$jscomp$inline_1722 = {
findFiberByHostInstance: getClosestInstanceFromNode,
bundleType: 0,
version: "19.0.0-www-classic-eeb06fc6",
version: "19.0.0-www-classic-8eec4eed",
rendererPackageName: "react-dom"
};
var internals$jscomp$inline_2150 = {
Expand Down Expand Up @@ -17100,7 +17100,7 @@ var internals$jscomp$inline_2150 = {
scheduleRoot: null,
setRefreshHandler: null,
getCurrentFiber: null,
reconcilerVersion: "19.0.0-www-classic-eeb06fc6"
reconcilerVersion: "19.0.0-www-classic-8eec4eed"
};
if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
var hook$jscomp$inline_2151 = __REACT_DEVTOOLS_GLOBAL_HOOK__;
Expand Down Expand Up @@ -17550,4 +17550,4 @@ exports.useFormState = function (action, initialState, permalink) {
exports.useFormStatus = function () {
return ReactCurrentDispatcher$2.current.useHostTransitionStatus();
};
exports.version = "19.0.0-www-classic-eeb06fc6";
exports.version = "19.0.0-www-classic-8eec4eed";
16 changes: 8 additions & 8 deletions compiled/facebook-www/ReactDOM-prod.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -15153,14 +15153,14 @@ function setInitialProperties(domElement, tag, props) {
for (defaultChecked in props)
props.hasOwnProperty(defaultChecked) &&
((propKey = props[defaultChecked]),
null != propKey &&
void 0 !== propKey &&
setPropOnCustomElement(
domElement,
tag,
defaultChecked,
propKey,
props,
null
void 0
));
return;
}
Expand Down Expand Up @@ -15458,13 +15458,13 @@ function updateProperties(domElement, tag, lastProps, nextProps) {
for (var propKey$249 in lastProps)
(propKey$223 = lastProps[propKey$249]),
lastProps.hasOwnProperty(propKey$249) &&
null != propKey$223 &&
void 0 !== propKey$223 &&
!nextProps.hasOwnProperty(propKey$249) &&
setPropOnCustomElement(
domElement,
tag,
propKey$249,
null,
void 0,
nextProps,
propKey$223
);
Expand All @@ -15473,7 +15473,7 @@ function updateProperties(domElement, tag, lastProps, nextProps) {
(propKey = lastProps[defaultChecked]),
!nextProps.hasOwnProperty(defaultChecked) ||
propKey$223 === propKey ||
(null == propKey$223 && null == propKey) ||
(void 0 === propKey$223 && void 0 === propKey) ||
setPropOnCustomElement(
domElement,
tag,
Expand Down Expand Up @@ -16586,7 +16586,7 @@ Internals.Events = [
var devToolsConfig$jscomp$inline_1683 = {
findFiberByHostInstance: getClosestInstanceFromNode,
bundleType: 0,
version: "19.0.0-www-modern-6d21bdba",
version: "19.0.0-www-modern-6b365d04",
rendererPackageName: "react-dom"
};
var internals$jscomp$inline_2112 = {
Expand Down Expand Up @@ -16616,7 +16616,7 @@ var internals$jscomp$inline_2112 = {
scheduleRoot: null,
setRefreshHandler: null,
getCurrentFiber: null,
reconcilerVersion: "19.0.0-www-modern-6d21bdba"
reconcilerVersion: "19.0.0-www-modern-6b365d04"
};
if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
var hook$jscomp$inline_2113 = __REACT_DEVTOOLS_GLOBAL_HOOK__;
Expand Down Expand Up @@ -16919,4 +16919,4 @@ exports.useFormState = function (action, initialState, permalink) {
exports.useFormStatus = function () {
return ReactCurrentDispatcher$2.current.useHostTransitionStatus();
};
exports.version = "19.0.0-www-modern-6d21bdba";
exports.version = "19.0.0-www-modern-6b365d04";
16 changes: 8 additions & 8 deletions compiled/facebook-www/ReactDOM-profiling.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -15595,14 +15595,14 @@ function setInitialProperties(domElement, tag, props) {
for (defaultChecked in props)
props.hasOwnProperty(defaultChecked) &&
((propKey = props[defaultChecked]),
null != propKey &&
void 0 !== propKey &&
setPropOnCustomElement(
domElement,
tag,
defaultChecked,
propKey,
props,
null
void 0
));
return;
}
Expand Down Expand Up @@ -15900,13 +15900,13 @@ function updateProperties(domElement, tag, lastProps, nextProps) {
for (var propKey$265 in lastProps)
(propKey$239 = lastProps[propKey$265]),
lastProps.hasOwnProperty(propKey$265) &&
null != propKey$239 &&
void 0 !== propKey$239 &&
!nextProps.hasOwnProperty(propKey$265) &&
setPropOnCustomElement(
domElement,
tag,
propKey$265,
null,
void 0,
nextProps,
propKey$239
);
Expand All @@ -15915,7 +15915,7 @@ function updateProperties(domElement, tag, lastProps, nextProps) {
(propKey = lastProps[defaultChecked]),
!nextProps.hasOwnProperty(defaultChecked) ||
propKey$239 === propKey ||
(null == propKey$239 && null == propKey) ||
(void 0 === propKey$239 && void 0 === propKey) ||
setPropOnCustomElement(
domElement,
tag,
Expand Down Expand Up @@ -17820,7 +17820,7 @@ Internals.Events = [
var devToolsConfig$jscomp$inline_1808 = {
findFiberByHostInstance: getClosestInstanceFromNode,
bundleType: 0,
version: "19.0.0-www-classic-567f79e4",
version: "19.0.0-www-classic-4051d1c8",
rendererPackageName: "react-dom"
};
(function (internals) {
Expand Down Expand Up @@ -17864,7 +17864,7 @@ var devToolsConfig$jscomp$inline_1808 = {
scheduleRoot: null,
setRefreshHandler: null,
getCurrentFiber: null,
reconcilerVersion: "19.0.0-www-classic-567f79e4"
reconcilerVersion: "19.0.0-www-classic-4051d1c8"
});
var ReactFiberErrorDialogWWW = require("ReactFiberErrorDialog");
if ("function" !== typeof ReactFiberErrorDialogWWW.showErrorDialog)
Expand Down Expand Up @@ -18301,7 +18301,7 @@ exports.useFormState = function (action, initialState, permalink) {
exports.useFormStatus = function () {
return ReactCurrentDispatcher$2.current.useHostTransitionStatus();
};
exports.version = "19.0.0-www-classic-567f79e4";
exports.version = "19.0.0-www-classic-4051d1c8";
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
"function" ===
typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop &&
Expand Down
16 changes: 8 additions & 8 deletions compiled/facebook-www/ReactDOM-profiling.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -15897,14 +15897,14 @@ function setInitialProperties(domElement, tag, props) {
for (defaultChecked in props)
props.hasOwnProperty(defaultChecked) &&
((propKey = props[defaultChecked]),
null != propKey &&
void 0 !== propKey &&
setPropOnCustomElement(
domElement,
tag,
defaultChecked,
propKey,
props,
null
void 0
));
return;
}
Expand Down Expand Up @@ -16202,13 +16202,13 @@ function updateProperties(domElement, tag, lastProps, nextProps) {
for (var propKey$270 in lastProps)
(propKey$244 = lastProps[propKey$270]),
lastProps.hasOwnProperty(propKey$270) &&
null != propKey$244 &&
void 0 !== propKey$244 &&
!nextProps.hasOwnProperty(propKey$270) &&
setPropOnCustomElement(
domElement,
tag,
propKey$270,
null,
void 0,
nextProps,
propKey$244
);
Expand All @@ -16217,7 +16217,7 @@ function updateProperties(domElement, tag, lastProps, nextProps) {
(propKey = lastProps[defaultChecked]),
!nextProps.hasOwnProperty(defaultChecked) ||
propKey$244 === propKey ||
(null == propKey$244 && null == propKey) ||
(void 0 === propKey$244 && void 0 === propKey) ||
setPropOnCustomElement(
domElement,
tag,
Expand Down Expand Up @@ -17330,7 +17330,7 @@ Internals.Events = [
var devToolsConfig$jscomp$inline_1769 = {
findFiberByHostInstance: getClosestInstanceFromNode,
bundleType: 0,
version: "19.0.0-www-modern-932b5800",
version: "19.0.0-www-modern-7a481094",
rendererPackageName: "react-dom"
};
(function (internals) {
Expand Down Expand Up @@ -17374,7 +17374,7 @@ var devToolsConfig$jscomp$inline_1769 = {
scheduleRoot: null,
setRefreshHandler: null,
getCurrentFiber: null,
reconcilerVersion: "19.0.0-www-modern-932b5800"
reconcilerVersion: "19.0.0-www-modern-7a481094"
});
var ReactFiberErrorDialogWWW = require("ReactFiberErrorDialog");
if ("function" !== typeof ReactFiberErrorDialogWWW.showErrorDialog)
Expand Down Expand Up @@ -17664,7 +17664,7 @@ exports.useFormState = function (action, initialState, permalink) {
exports.useFormStatus = function () {
return ReactCurrentDispatcher$2.current.useHostTransitionStatus();
};
exports.version = "19.0.0-www-modern-932b5800";
exports.version = "19.0.0-www-modern-7a481094";
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
"function" ===
typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop &&
Expand Down
Loading

0 comments on commit 385c181

Please sign in to comment.