[labs/react] Fix for #2799: add support for properties with custom ac…

…cessors (#2800)

* fix(labs/react): #2799 - add support for properties with custom accessors

* Add changeset

Co-authored-by: Justin Fagnani <justinfagnani@google.com>

.changeset/twelve-lobsters-join.md

@@ -0,0 +1,5 @@
+---
+'@lit-labs/react': patch
+---
+
+Support setting custom accessors by using an 'in' check instead of a for/in loop to check for properties.

packages/labs/react/src/create-component.ts

@@ -130,6 +130,7 @@ export const createComponent = <I extends HTMLElement, E extends Events = {}>(
 ) => {
   const Component = React.Component;
   const createElement = React.createElement;
+  const eventProps = new Set(Object.keys(events ?? {}));
 
   // Props the user is allowed to use, includes standard attributes, children,
   // ref, as well as special event and element properties.
@@ -147,28 +148,6 @@ export const createComponent = <I extends HTMLElement, E extends Events = {}>(
     __forwardedRef?: React.Ref<unknown>;
   };
 
-  // Set of properties/events which should be specially handled by the wrapper
-  // and not handled directly by React.
-  const elementClassProps = new Set(Object.keys(events ?? {}));
-  for (const p in elementClass.prototype) {
-    if (!(p in HTMLElement.prototype)) {
-      if (reservedReactProperties.has(p)) {
-        // Note, this effectively warns only for `ref` since the other
-        // reserved props are on HTMLElement.prototype. To address this
-        // would require crawling down the prototype, which doesn't feel worth
-        // it since implementing these properties on an element is extremely
-        // rare.
-        console.warn(
-          `${tagName} contains property ${p} which is a React ` +
-            `reserved property. It will be used by React and not set on ` +
-            `the element.`
-        );
-      } else {
-        elementClassProps.add(p);
-      }
-    }
-  }
-
   class ReactComponent extends Component<ComponentProps> {
     private _element: I | null = null;
     private _elementProps!: {[index: string]: unknown};
@@ -247,7 +226,12 @@ export const createComponent = <I extends HTMLElement, E extends Events = {}>(
       for (const [k, v] of Object.entries(this.props)) {
         if (k === '__forwardedRef') continue;
 
-        if (elementClassProps.has(k)) {
+        if (
+          eventProps.has(k) ||
+          (!reservedReactProperties.has(k) &&
+            !(k in HTMLElement.prototype) &&
+            k in elementClass.prototype)
+        ) {
           this._elementProps[k] = v;
         } else {
           // React does *not* handle `className` for custom elements so

packages/labs/react/src/test/create-component_test.tsx

@@ -18,6 +18,10 @@ import {assert} from '@esm-bundle/chai';
 // Needed for JSX expressions
 const React = window.React;
 
+interface Foo {
+  foo?: boolean;
+}
+
 const elementName = 'basic-element';
 @customElement(elementName)
 class BasicElement extends ReactiveElement {
@@ -43,6 +47,17 @@ class BasicElement extends ReactiveElement {
   @property({type: Array, reflect: true})
   rarr: unknown[] | null | undefined = null;
 
+  @property({ type: Object })
+  set customAccessors(customAccessors: Foo) {
+    const oldValue = this._customAccessors;
+    this._customAccessors = customAccessors;
+    this.requestUpdate("customAccessors", oldValue);
+  }
+  get customAccessors(): Foo {
+    return this._customAccessors;
+  }
+  private _customAccessors = {};
+
   fire(name: string) {
     this.dispatchEvent(new Event(name));
   }
@@ -207,12 +222,14 @@ suite('createComponent', () => {
       num: 5,
       obj: o,
       arr: a,
+      customAccessors: o
     });
     assert.equal(el.bool, true);
     assert.equal(el.str, 'str');
     assert.equal(el.num, 5);
     assert.deepEqual(el.obj, o);
     assert.deepEqual(el.arr, a);
+    assert.deepEqual(el.customAccessors, o);
     const firstEl = el;
     // update
     o = {foo: false};
@@ -223,13 +240,15 @@ suite('createComponent', () => {
       num: 10,
       obj: o,
       arr: a,
+      customAccessors: o
     });
     assert.equal(firstEl, el);
     assert.equal(el.bool, false);
     assert.equal(el.str, 'str2');
     assert.equal(el.num, 10);
     assert.deepEqual(el.obj, o);
     assert.deepEqual(el.arr, a);
+    assert.deepEqual(el.customAccessors, o);
   });
 
   test('can set properties that reflect', async () => {
@@ -364,21 +383,4 @@ suite('createComponent', () => {
     assert.equal(el.style.display, 'block');
     assert.equal(el.getAttribute('class'), 'foo bar');
   });
-
-  test('warns if element contains reserved props', async () => {
-    const warn = console.warn;
-    let warning: string;
-    console.warn = (m: string) => {
-      warning = m;
-    };
-    const tag = 'x-warn';
-    @customElement(tag)
-    class Warn extends ReactiveElement {
-      @property()
-      ref = 'hi';
-    }
-    createComponent(window.React, tag, Warn);
-    assert.include(warning!, 'ref');
-    console.warn = warn;
-  });
 });