diff --git a/.changeset/strange-peas-design.md b/.changeset/strange-peas-design.md new file mode 100644 index 0000000000..c7a58f35d1 --- /dev/null +++ b/.changeset/strange-peas-design.md @@ -0,0 +1,13 @@ +--- +'@lit/react': patch +--- + +Wrapped components will now keep track of JSX props from previous render that were set as a property on the element, but are now missing, and set the property to `undefined`. Note, wrapped components still do not have "default props" and missing props will be treated the same as explicitly passing in `undefined`. + +This fixes the previously unexpected behavior where the following JSX when first rendered with a truthy condition + +```jsx +return condition ? : ; +``` + +would leave the `disabled` property and reflected attribute to be `true` even when the condition turns falsey. diff --git a/package.json b/package.json index fd8e1de5b1..3b60029039 100644 --- a/package.json +++ b/package.json @@ -171,7 +171,6 @@ "./packages/labs/gen-wrapper-vue:test", "./packages/labs/nextjs:test", "./packages/labs/preact-signals:test", - "./packages/labs/react:test", "./packages/labs/rollup-plugin-minify-html-literals:test", "./packages/labs/ssr:test", "./packages/labs/ssr-dom-shim:test", diff --git a/packages/react/src/create-component.ts b/packages/react/src/create-component.ts index fd4fea5f1b..2cb587dea9 100644 --- a/packages/react/src/create-component.ts +++ b/packages/react/src/create-component.ts @@ -243,7 +243,7 @@ export const createComponent = < type Props = ComponentProps; const ReactComponent = React.forwardRef((props, ref) => { - const prevPropsRef = React.useRef(null); + const prevElemPropsRef = React.useRef(new Map()); const elementRef = React.useRef(null); // Props to be passed to React.createElement @@ -274,20 +274,26 @@ export const createComponent = < if (elementRef.current === null) { return; } - for (const prop in elementProps) { + const newElemProps = new Map(); + for (const key in elementProps) { setProperty( elementRef.current, - prop, - props[prop], - prevPropsRef.current ? prevPropsRef.current[prop] : undefined, + key, + props[key], + prevElemPropsRef.current.get(key), events ); + prevElemPropsRef.current.delete(key); + newElemProps.set(key, props[key]); } - // Note, the spirit of React might be to "unset" any old values that - // are no longer included; however, there's no reasonable value to set - // them to so we just leave the previous state as is. - - prevPropsRef.current = props; + // "Unset" any props from previous render that no longer exist. + // Setting to `undefined` seems like the correct thing to "unset" + // but currently React will set it as `null`. + // See https://github.com/facebook/react/issues/28203 + for (const [key, value] of prevElemPropsRef.current) { + setProperty(elementRef.current, key, undefined, value, events); + } + prevElemPropsRef.current = newElemProps; }); // Empty dependency array so this will only run once after first render. diff --git a/packages/react/src/test/create-component_test.tsx b/packages/react/src/test/create-component_test.tsx index 70a851d96d..5b12f6ebbe 100644 --- a/packages/react/src/test/create-component_test.tsx +++ b/packages/react/src/test/create-component_test.tsx @@ -334,10 +334,7 @@ suite('createComponent', () => { assert.equal(el.getAttribute('id'), 'foo'); assert.equal(el.id, 'foo'); - // We currently require passing `undefined` to "unset" rather than - // omitting the prop - // See https://github.com/lit/lit/issues/4227 - render(); + render(); assert.equal(el.getAttribute('id'), null); assert.equal(el.id, ''); @@ -383,7 +380,7 @@ suite('createComponent', () => { assert.equal(el.getAttribute('hidden'), ''); assert.equal(el.hidden, true); - render(