Skip to content

Commit

Permalink
Fix merge props (#1764)
Browse files Browse the repository at this point in the history
* test: add tests to `mergeProps`

* refactor: comment unused code

* fix: override prototyped props and ignore undefined props in `mergeProps`
  • Loading branch information
juanrgm committed Jun 26, 2023
1 parent 93d44d4 commit 8ba0e80
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 10 deletions.
5 changes: 5 additions & 0 deletions .changeset/pretty-points-cross.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"solid-js": patch
---

Fix `mergeProps`.
20 changes: 11 additions & 9 deletions packages/solid/src/render/component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -224,33 +224,35 @@ export function mergeProps<T extends unknown[]>(...sources: T): MergeProps<T> {
}
const target: Record<string, any> = {};
const sourcesMap: Record<string, any[]> = {};
let someNonTargetKey = false;
const defined = new Set<string>()
//let someNonTargetKey = false;

for (let i = sources.length - 1; i >= 0; i--) {
const source = sources[i] as Record<string, any>;
if (!source) continue;
const sourceKeys = Object.getOwnPropertyNames(source);
someNonTargetKey = someNonTargetKey || (i !== 0 && !!sourceKeys.length);
//someNonTargetKey = someNonTargetKey || (i !== 0 && !!sourceKeys.length);
for (let i = 0, length = sourceKeys.length; i < length; i++) {
const key = sourceKeys[i];
if (key === "__proto__" || key === "constructor") {
if (key === "__proto__" || key === "constructor")
continue;
} else if (!(key in target)) {
const desc = Object.getOwnPropertyDescriptor(source, key)!;
const desc = Object.getOwnPropertyDescriptor(source, key)!;
if (!defined.has(key)) {
if (desc.get) {
defined.add(key);
Object.defineProperty(target, key, {
enumerable: true,
// [breaking && performance]
// configurable: false,
configurable: true,
get: resolveSources.bind(
(sourcesMap[key] = [desc.get.bind(source)])
),
});
} else target[key] = desc.value;
} else {
if (desc.value !== undefined) defined.add(key);
target[key] = desc.value;
}
} else {
const sources = sourcesMap[key];
const desc = Object.getOwnPropertyDescriptor(source, key)!;
if (sources) {
if (desc.get) {
sources.push(desc.get.bind(source));
Expand Down
15 changes: 14 additions & 1 deletion packages/solid/test/component.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,20 @@ describe("mergeProps", () => {
const a = { value: 1 }
const b = { get value() { return bValue } }
const c = { get value() { return undefined } }
const props = mergeProps(a, b, c)
const d = { value: undefined }
const props = mergeProps(a, b, c, d)
expect(props.value).toBe(1)
bValue = 2;
expect(props.value).toBe(2)
});
it("includes undefined property", () => {
const value = { a: undefined }
const getter = { get a() { return undefined; } }
expect("a" in mergeProps(value)).toBeTruthy()
expect("a" in mergeProps(getter)).toBeTruthy()
expect("a" in mergeProps(value, getter)).toBeTruthy()
expect("a" in mergeProps(getter, value)).toBeTruthy()
});
it("doesn't keep references for non-getters", () => {
const a = { value1: 1 }
const b = { value2: 2 }
Expand Down Expand Up @@ -168,6 +177,10 @@ describe("mergeProps", () => {
mergeProps({ value: 1 }, JSON.parse('{ "prototype": { "evil": true } }'))
expect(({} as any).evil).toBeUndefined()
});
it("sets already prototyped properties", () => {
expect(mergeProps({ toString: 1 }).toString).toBe(1)
expect(({}).toString).toBeTypeOf("function")
});
});

describe("Set Default Props", () => {
Expand Down

0 comments on commit 8ba0e80

Please sign in to comment.