diff --git a/.changeset/seven-phones-admire.md b/.changeset/seven-phones-admire.md new file mode 100644 index 000000000..8e37e6a1f --- /dev/null +++ b/.changeset/seven-phones-admire.md @@ -0,0 +1,5 @@ +--- +"solid-js": patch +--- + +fix #1971 order of merged properties diff --git a/packages/solid/src/render/component.ts b/packages/solid/src/render/component.ts index 098c9f9e0..66321689b 100644 --- a/packages/solid/src/render/component.ts +++ b/packages/solid/src/render/component.ts @@ -222,9 +222,8 @@ export function mergeProps(...sources: T): MergeProps { propTraps ) as unknown as MergeProps; } - const target: Record = {}; const sourcesMap: Record = {}; - const defined = new Set(); + const defined: Record = Object.create(null); //let someNonTargetKey = false; for (let i = sources.length - 1; i >= 0; i--) { @@ -232,34 +231,37 @@ export function mergeProps(...sources: T): MergeProps { 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 = {}; + 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; diff --git a/packages/solid/test/component.spec.ts b/packages/solid/test/component.spec.ts index b28156b7a..a8a70558a 100644 --- a/packages/solid/test/component.spec.ts +++ b/packages/solid/test/component.spec.ts @@ -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 }; @@ -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( @@ -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);