Skip to content

Commit

Permalink
fix #1971 order of merged properties
Browse files Browse the repository at this point in the history
  • Loading branch information
ryansolid committed Jan 8, 2024
1 parent 80d4830 commit 71bea78
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 22 deletions.
5 changes: 5 additions & 0 deletions .changeset/seven-phones-admire.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"solid-js": patch
---

fix #1971 order of merged properties
44 changes: 23 additions & 21 deletions packages/solid/src/render/component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -222,44 +222,46 @@ export function mergeProps<T extends unknown[]>(...sources: T): MergeProps<T> {
propTraps
) as unknown as MergeProps<T>;
}
const target: Record<string, any> = {};
const sourcesMap: Record<string, any[]> = {};
const defined = new Set<string>();
const defined: Record<string, PropertyDescriptor | undefined> = Object.create(null);
//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);
for (let i = 0, length = sourceKeys.length; i < length; i++) {
for (let i = sourceKeys.length - 1; i >= 0; i--) {
const key = sourceKeys[i];
if (key === "__proto__" || key === "constructor") continue;
const desc = Object.getOwnPropertyDescriptor(source, key)!;
if (!defined.has(key)) {
if (desc.get) {
defined.add(key);
Object.defineProperty(target, key, {
enumerable: true,
configurable: true,
get: resolveSources.bind((sourcesMap[key] = [desc.get.bind(source)]))
});
} else {
if (desc.value !== undefined) defined.add(key);
target[key] = desc.value;
}
if (!defined[key]) {
defined[key] = desc.get
? {
enumerable: true,
configurable: true,
get: resolveSources.bind((sourcesMap[key] = [desc.get.bind(source)]))
}
: desc.value !== undefined
? desc
: undefined;
} else {
const sources = sourcesMap[key];
if (sources) {
if (desc.get) {
sources.push(desc.get.bind(source));
} else if (desc.value !== undefined) {
sources.push(() => desc.value);
}
} else if (target[key] === undefined) target[key] = desc.value;
if (desc.get) sources.push(desc.get.bind(source));
else if (desc.value !== undefined) sources.push(() => desc.value);
}
}
}
}
const target: Record<string, any> = {};
const definedKeys = Object.keys(defined);
for (let i = definedKeys.length - 1; i >= 0; i--) {
const key = definedKeys[i],
desc = defined[key];
if (desc && desc.get) Object.defineProperty(target, key, desc);
else target[key] = desc ? desc.value : undefined;
}
// [breaking && performance]
//return (someNonTargetKey ? target : sources[0]) as any;
return target as any;
Expand Down
4 changes: 3 additions & 1 deletion packages/solid/test/component.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ describe("mergeProps", () => {
a.value1 = b.value2 = 3;
expect(props.value1).toBe(1);
expect(props.value2).toBe(2);
expect(Object.keys(props).join()).toBe("value1,value2");
});
it("without getter transfers only value", () => {
const a = { value1: 1 };
Expand All @@ -111,6 +112,7 @@ describe("mergeProps", () => {
const props = mergeProps(a, b);
a.value1 = 3;
expect(props.value1).toBe(1);
expect(Object.keys(props).join()).toBe("value1,value2");
});
it("overrides enumerables", () => {
const a = Object.defineProperties(
Expand Down Expand Up @@ -204,7 +206,7 @@ describe("mergeProps", () => {
});
it("works with a array source", () => {
const props = mergeProps({ value: 1 }, [2]);
expect(Object.keys(props).join()).toBe("0,length,value");
expect(Object.keys(props).join()).toBe("0,value,length");
expect(props.value).toBe(1);
expect(props.length).toBe(1);
expect(props[0]).toBe(2);
Expand Down

0 comments on commit 71bea78

Please sign in to comment.