Skip to content

Commit 800452a

Browse files
committed
Fix redundant setAttribute calls
1 parent 41564dc commit 800452a

3 files changed

Lines changed: 49 additions & 4 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@preact/signals": patch
3+
---
4+
5+
Fix redundant DOM attribute writes when a parent rerenders with unchanged signal props. The DIFFED hook no longer writes Signal references back into `vnode.props`, which was causing Preact's prop diff to see a mismatch (old: Signal, new: peeked value) and re-apply every signal-bound attribute on every parent rerender. Visibly broken for canvas `width`/`height`, which clear the bitmap on every assignment per the HTML spec.

packages/preact/src/index.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -294,10 +294,6 @@ hook(OptionsTypes.DIFFED, (old, vnode) => {
294294
updater._update(signal, renderedProps);
295295
}
296296
}
297-
298-
for (let prop in props) {
299-
renderedProps[prop] = props[prop];
300-
}
301297
}
302298
}
303299
old(vnode);
@@ -365,6 +361,16 @@ hook(OptionsTypes.UNMOUNT, (old, vnode: VNode) => {
365361
}
366362
}
367363
}
364+
// Restore Signal references into vnode.props so that, if this vnode
365+
// instance is reused for a remount, the DIFF hook can re-detect the
366+
// signal-bound props (they were replaced with peeked values during diff).
367+
let signalProps = vnode.__np;
368+
if (signalProps) {
369+
let props = vnode.props;
370+
for (let prop in signalProps) {
371+
props[prop] = signalProps[prop];
372+
}
373+
}
368374
vnode.__np = undefined;
369375
} else {
370376
let component = vnode.__c;

packages/preact/test/browser/index.test.tsx

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -788,6 +788,40 @@ describe("@preact/signals", () => {
788788
expect(div.getAttribute("title")).to.equal("");
789789
expect(spy).not.toHaveBeenCalled();
790790
});
791+
792+
// https://github.com/preactjs/signals/issues/923
793+
it("should not re-apply unchanged signal props when parent rerenders", () => {
794+
const count = signal(0);
795+
const width = signal(200);
796+
797+
function App() {
798+
return (
799+
<div>
800+
<p>{count.value}</p>
801+
{/* @ts-ignore */}
802+
<canvas width={width} />
803+
</div>
804+
);
805+
}
806+
807+
render(<App />, scratch);
808+
809+
const canvas = scratch.querySelector("canvas") as HTMLCanvasElement;
810+
const setAttributeSpy = vi.spyOn(canvas, "setAttribute");
811+
812+
act(() => {
813+
count.value++;
814+
});
815+
816+
// Parent rerendered but width signal didn't change — Preact must
817+
// not re-apply the attribute, otherwise non-idempotent setters
818+
// like canvas width/height (which reset the bitmap to transparent
819+
// even when the value is unchanged) would be triggered redundantly.
820+
const widthCalls = setAttributeSpy.mock.calls.filter(
821+
([name]) => name === "width"
822+
);
823+
expect(widthCalls).to.have.length(0);
824+
});
791825
});
792826

793827
describe("hooks mixed with signals", () => {

0 commit comments

Comments
 (0)