Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions eslint.config.js → eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
Expand Down
2 changes: 1 addition & 1 deletion package-scripts.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ module.exports = {
),
size: {
description: "check the size of the bundle",
script: "bundlesize",
script: "size-limit",
},
},
build: {
Expand Down
11 changes: 6 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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": {
Expand Down
File renamed without changes.
72 changes: 72 additions & 0 deletions src/renderComponent.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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");
});
});
21 changes: 14 additions & 7 deletions src/renderComponent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading