From ba0c040cd6864b058b4d2eb05498edb0b9ca5f1e Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 2 Apr 2024 10:37:01 -0400 Subject: [PATCH] Differentiate null and undefined in Custom Elements Deleting a property now means setting it to undefined. --- .../src/client/ReactDOMComponent.js | 10 +-- .../__tests__/DOMPropertyOperations-test.js | 73 ++++++++++++++++++- 2 files changed, 76 insertions(+), 7 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js index 715b241cdb02f..36a59f0850e7f 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMComponent.js +++ b/packages/react-dom-bindings/src/client/ReactDOMComponent.js @@ -1301,7 +1301,7 @@ export function setInitialProperties( continue; } const propValue = props[propKey]; - if (propValue == null) { + if (propValue === undefined) { continue; } setPropOnCustomElement( @@ -1310,7 +1310,7 @@ export function setInitialProperties( propKey, propValue, props, - null, + undefined, ); } return; @@ -1741,14 +1741,14 @@ export function updateProperties( const lastProp = lastProps[propKey]; if ( lastProps.hasOwnProperty(propKey) && - lastProp != null && + lastProp !== undefined && !nextProps.hasOwnProperty(propKey) ) { setPropOnCustomElement( domElement, tag, propKey, - null, + undefined, nextProps, lastProp, ); @@ -1760,7 +1760,7 @@ export function updateProperties( if ( nextProps.hasOwnProperty(propKey) && nextProp !== lastProp && - (nextProp != null || lastProp != null) + (nextProp !== undefined || lastProp !== undefined) ) { setPropOnCustomElement( domElement, diff --git a/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js b/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js index 1ca571df71cc8..e6565d0097267 100644 --- a/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js +++ b/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js @@ -1230,7 +1230,7 @@ describe('DOMPropertyOperations', () => { }); customelement.dispatchEvent(new Event('customevent')); expect(oncustomevent).toHaveBeenCalledTimes(2); - expect(customelement.oncustomevent).toBe(null); + expect(customelement.oncustomevent).toBe(undefined); expect(customelement.getAttribute('oncustomevent')).toBe(null); }); @@ -1290,12 +1290,33 @@ describe('DOMPropertyOperations', () => { await act(() => { root.render(); }); - expect(customElement.foo).toBe(null); + expect(customElement.foo).toBe(undefined); await act(() => { root.render(); }); expect(customElement.foo).toBe(myFunction); }); + + it('switching between null and undefined should update a property', async () => { + const container = document.createElement('div'); + document.body.appendChild(container); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render(); + }); + const customElement = container.querySelector('my-custom-element'); + customElement.foo = undefined; + + await act(() => { + root.render(); + }); + expect(customElement.foo).toBe(null); + + await act(() => { + root.render(); + }); + expect(customElement.foo).toBe(undefined); + }); }); describe('deleteValueForProperty', () => { @@ -1349,5 +1370,53 @@ describe('DOMPropertyOperations', () => { }); expect(container.firstChild.getAttribute('size')).toBe('5px'); }); + + it('custom elements should remove by setting undefined to restore defaults', async () => { + const container = document.createElement('div'); + document.body.appendChild(container); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render(); + }); + const customElement = container.querySelector('my-custom-element'); + + // Non-setter but existing property to active the `in` heuristic + customElement.raw = 1; + + // Install a setter to activate the `in` heuristic + Object.defineProperty(customElement, 'object', { + set: function (value = null) { + this._object = value; + }, + get: function () { + return this._object; + }, + }); + + Object.defineProperty(customElement, 'string', { + set: function (value = '') { + this._string = value; + }, + get: function () { + return this._string; + }, + }); + + const obj = {}; + await act(() => { + root.render(); + }); + expect(customElement.raw).toBe(2); + expect(customElement.object).toBe(obj); + expect(customElement.string).toBe('hi'); + + // Removing the properties should reset to defaults by passing undefined + await act(() => { + root.render(); + }); + expect(customElement.raw).toBe(undefined); + expect(customElement.object).toBe(null); + expect(customElement.string).toBe(''); + }); }); });