Skip to content

fix(preact): stale prop updater keeps writing to the DOM after all signal props are removed - #948

Merged
JoviDeCroock merged 1 commit into
mainfrom
fix/dispose-removed-prop-updaters
Jul 5, 2026
Merged

fix(preact): stale prop updater keeps writing to the DOM after all signal props are removed#948
JoviDeCroock merged 1 commit into
mainfrom
fix/dispose-removed-prop-updaters

Conversation

@JoviDeCroock

Copy link
Copy Markdown
Member

Root cause

In the DIFFED hook, the loop that disposes updaters for props no longer bound to a signal is nested inside if (props) where props = vnode.__np. When a re-render passes no signal props for the element — e.g. <input value={editing ? draft : "saved"} /> flipping to the plain-string branch — vnode.__np is undefined and the whole block, including disposal, is skipped. The old PropertyUpdater effect stays subscribed to the previous signal and keeps writing its values straight into the DOM property, clobbering whatever Preact rendered, until the element unmounts.

Fix

Run the disposal pass over dom._updaters unconditionally, treating a missing vnode.__np the same as "no props are signal-bound anymore", and only lazily create the updaters map when the new render actually has signal props. The regression test renders <input value={signal} />, re-renders with a plain string, then writes to the signal and asserts the DOM keeps the rendered value.

Note: this PR was authored by Claude and @JoviDeCroock.

…rops

The DIFFED hook only disposed stale updaters inside the vnode.__np
branch, so a re-render that carried no signal props at all skipped
disposal entirely. The orphaned updater effect kept writing the old
signal's values into the DOM until unmount.
@changeset-bot

changeset-bot Bot commented Jul 5, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 60f50ca

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@preact/signals Patch
preact-signals-devtools Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@netlify

netlify Bot commented Jul 5, 2026

Copy link
Copy Markdown

Deploy Preview for preact-signals-demo ready!

Name Link
🔨 Latest commit 60f50ca
🔍 Latest deploy log https://app.netlify.com/projects/preact-signals-demo/deploys/6a49fefa39c60b00086f5eff
😎 Deploy Preview https://deploy-preview-948--preact-signals-demo.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Size Change: +24 B (+0.01%)

Total Size: 187 kB

📦 View Changed
Filename Size Change
docs/dist/assets/bench-********.js 1.59 kB -4 B (-0.25%)
docs/dist/assets/devtools-********.js 911 B +3 B (+0.33%)
docs/dist/assets/EmbeddedDevtools-********.js 17.7 kB +8 B (+0.05%)
docs/dist/assets/index-********.js 8.43 kB +1 B (+0.01%)
docs/dist/assets/signals.module-********.js 2.65 kB +1 B (+0.04%)
docs/dist/assets/Unmount-********.js 652 B +2 B (+0.31%)
docs/dist/assets/utils.module-********.js 501 B +1 B (+0.2%)
docs/dist/basic-********.js 244 B -1 B (-0.41%)
packages/devtools-ui/dist/devtools-ui.js 16.2 kB +3 B (+0.02%)
packages/devtools-ui/dist/devtools-ui.mjs 15.6 kB +8 B (+0.05%)
packages/preact/dist/signals.js 1.81 kB +4 B (+0.22%)
packages/preact/dist/signals.mjs 1.75 kB -2 B (-0.11%)
ℹ️ View Unchanged
Filename Size
docs/dist/assets/client-********.js 46.6 kB
docs/dist/assets/jsxRuntime.module-********.js 300 B
docs/dist/assets/preact.module-********.js 4.74 kB
docs/dist/assets/signals-core.module-********.js 1.89 kB
docs/dist/assets/style-********.css 5.26 kB
docs/dist/nesting-********.js 1.14 kB
docs/dist/react-********.js 243 B
packages/core/dist/signals-core.js 1.92 kB
packages/core/dist/signals-core.mjs 1.91 kB
packages/debug/dist/debug.js 4.64 kB
packages/debug/dist/debug.mjs 4.15 kB
packages/devtools-adapter/dist/devtools-adapter.js 2.36 kB
packages/devtools-adapter/dist/devtools-adapter.mjs 2.07 kB
packages/preact-transform/dist/signals-transform.js 1.3 kB
packages/preact-transform/dist/signals-transform.mjs 1.29 kB
packages/preact-transform/dist/signals-transform.umd.js 1.42 kB
packages/react-transform/dist/signals-transform.js 7.28 kB
packages/react-transform/dist/signals-transform.mjs 6.47 kB
packages/react-transform/dist/signals-transform.umd.js 7.39 kB
packages/react/dist/signals.js 214 B
packages/react/dist/signals.mjs 165 B
packages/vite-plugin/dist/vite-plugin.js 8.86 kB
packages/vite-plugin/dist/vite-plugin.mjs 7.86 kB

compressed-size-action

@JoviDeCroock
JoviDeCroock merged commit 6b0a76c into main Jul 5, 2026
6 checks passed
@JoviDeCroock
JoviDeCroock deleted the fix/dispose-removed-prop-updaters branch July 5, 2026 16:35
@github-actions github-actions Bot mentioned this pull request Jul 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants