Fix #1055: Prevent overwriting getter-only properties in renderComponent (#1056)

* replace bundlesize with size-limit to remove iltorb

* use mjs for config files

* add eslint config for mjs

* Fix #1055: Prevent overwriting getter-only properties in renderComponent

The component branch of renderComponent was using Object.assign(lazyProps, rest)
which attempts to overwrite getter-only properties (like 'active'). This causes
'Cannot set property active' errors, especially with React 19.

Fix: Use the same safe property descriptor merging pattern already used in the
render and children branches - only add properties that don't conflict with
existing getters.

Tests: Added comprehensive coverage for getter-only property scenarios across
all render paths (component, render, children). Tests fail with buggy code,
pass with fix.

Fixes: #1055

---------

Co-authored-by: joshua-burbidge <joshdburbidge@gmail.com>
Co-authored-by: Erik Rasmussen <erik@mini.local>

Author: 3 people

eslint.config.js eslint.config.mjseslint.config.js renamed to eslint.config.mjs

@@ -161,6 +161,21 @@ export default [
     },
   },
 
+  // Configuration for .mjs files (ES modules in Node.js environment)
+  {
+    files: ["**/*.mjs"],
+    languageOptions: {
+      ecmaVersion: "latest",
+      sourceType: "module",
+      globals: {
+        ...nodeGlobals,
+      },
+    },
+    rules: {
+      "no-undef": "error",
+    },
+  },
+
   // Specific overrides for ALL test files (JS and TS)
   {
     files: ["**/*.test.js", "**/*.test.jsx", "**/*.test.ts", "**/*.test.tsx"],

package-scripts.js

@@ -16,7 +16,7 @@ module.exports = {
       ),
       size: {
         description: "check the size of the bundle",
-        script: "bundlesize",
+        script: "size-limit",
       },
     },
     build: {

package.json

@@ -53,7 +53,8 @@
     "babel-core": "^7.0.0-bridge.0",
     "babel-eslint": "^10.1.0",
     "babel-jest": "^29.7.0",
-    "bundlesize": "^0.18.2",
+    "@size-limit/preset-small-lib": "^11.1.6",
+    "size-limit": "^11.1.6",
     "doctoc": "^2.2.1",
     "dtslint": "^4.2.1",
     "eslint": "^9.27.0",
@@ -100,18 +101,18 @@
       "/typescript/"
     ]
   },
-  "bundlesize": [
+  "size-limit": [
     {
       "path": "dist/react-final-form.umd.min.js",
-      "threshold": "2kB"
+      "limit": "4kB"
     },
     {
       "path": "dist/react-final-form.es.js",
-      "threshold": "3kB"
+      "limit": "4kB"
     },
     {
       "path": "dist/react-final-form.cjs.js",
-      "threshold": "3kB"
+      "limit": "4kB"
     }
   ],
   "collective": {

rollup.config.js rollup.config.mjsrollup.config.js renamed to rollup.config.mjs

@@ -69,4 +69,76 @@ describe("renderComponent", () => {
     expect(getA).not.toHaveBeenCalled();
     expect(getB).not.toHaveBeenCalled();
   });
+
+  it("should not overwrite getter-only properties when using component prop", () => {
+    // This test reproduces issue #1055
+    // When lazyProps has getter-only properties (like 'active'), and props contains
+    // a property with the same name, it should not attempt to overwrite the getter
+    const Component = () => null;
+    const props = {
+      component: Component,
+      active: "value-from-props", // This would cause "Cannot set property active" error
+      customProp: "custom",
+    };
+    const lazyProps = {};
+    Object.defineProperty(lazyProps, "active", {
+      get: () => "value-from-getter",
+      enumerable: true,
+      // Note: no setter - this is getter-only
+    });
+    const name = "TestComponent";
+
+    // Should not throw "Cannot set property active"
+    let result;
+    expect(() => {
+      result = renderComponent(props, lazyProps, name);
+    }).not.toThrow();
+
+    // Check the React element was created with correct props
+    expect(result.type).toBe(Component);
+    
+    // The getter-only property should remain and use the getter value
+    expect(result.props.active).toBe("value-from-getter");
+
+    // Custom props should still be passed through
+    expect(result.props.customProp).toBe("custom");
+  });
+
+  it("should handle getter-only properties in all render paths", () => {
+    const lazyProps = {};
+    Object.defineProperty(lazyProps, "active", {
+      get: () => "getter-value",
+      enumerable: true,
+    });
+
+    // Test with render prop
+    const render = jest.fn();
+    renderComponent(
+      { render, active: "prop-value" },
+      lazyProps,
+      "TestComponent",
+    );
+    expect(render).toHaveBeenCalled();
+    expect(render.mock.calls[0][0].active).toBe("getter-value");
+
+    // Test with children function
+    const children = jest.fn();
+    renderComponent(
+      { children, active: "prop-value" },
+      lazyProps,
+      "TestComponent",
+    );
+    expect(children).toHaveBeenCalled();
+    expect(children.mock.calls[0][0].active).toBe("getter-value");
+
+    // Test with component prop
+    const Component = () => null;
+    const result = renderComponent(
+      { component: Component, active: "prop-value" },
+      lazyProps,
+      "TestComponent",
+    );
+    expect(result.type).toBe(Component);
+    expect(result.props.active).toBe("getter-value");
+  });
 });

src/renderComponent.test.js

@@ -10,13 +10,20 @@ export default function renderComponent<T>(
 ): React.ReactNode {
   const { render, children, component, ...rest } = props;
   if (component) {
-    return React.createElement(
-      component,
-      Object.assign(lazyProps, rest, {
-        children,
-        render,
-      }),
-    );
+    // FIX: Don't use Object.assign which tries to overwrite getters
+    // Instead, create a new object with lazyProps descriptors first,
+    // then add non-conflicting properties from rest
+    const result = {} as any;
+    Object.defineProperties(result, Object.getOwnPropertyDescriptors(lazyProps));
+    const restDescriptors = Object.getOwnPropertyDescriptors(rest);
+    for (const key in restDescriptors) {
+      if (!(key in result)) {
+        Object.defineProperty(result, key, restDescriptors[key]);
+      }
+    }
+    result.children = children;
+    result.render = render;
+    return React.createElement(component, result);
   }
   if (render) {
     const result = {} as T;