Repository navigation
Conversation
🦋 Changeset detectedLatest commit: 6ec96cf The changes in this PR will be included in the next version bump. This PR includes changesets to release 15 packages
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 |
Merging this PR will degrade performance by 11.91%3 warnings
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing Footnotes
|
Summary
A delegated event keeps an unmounted component alive in Chrome for the life of the page.
eventHandlersimulatedcurrentTargetwith an accessor defined on each event, and each event got a new getter closure. V8 keeps the first such getter in the descriptor array that every event of that class shares. A heap snapshot of a page after one keydown shows the getter on theKeyboardEventmap, and its closure holds the event, the event's target and the nodes of the walk. #4325 deletes the override when the walk ends, but the delete does not remove the getter from the shared shape, so rc.21 still leaks.currentTargetis now a configurable data property. It is defined right before each delegated handler runs, through adefine(name, value)helper that replacesretarget, so no getter is created. The deletes from #4325 stay, and native listeners that run later in the dispatch still read the native values. Each client scenario is 42 B smaller minified.I found this through vercel-labs/ui, which carries a pnpm patch on
@solidjs/webthat shares one getter across events. Its Solid leak spec mounts and unmounts each component 50 to 200 times and counts detached DOM nodes and heap growth. On rc.21 without that patch, 4 of its 47 tests fail on a built playground: combobox, slider, resizable, and tabs after an arrow key.How did you test this change?
The new test in
packages/web/test/delegated-event-restore.spec.tsxreads the event's owncurrentTargetdescriptor inside two nested delegated handlers. It expects no getter and the handler's own node as the value. It fails onnext, where the descriptor has a getter.After
pnpm build, inpackages/web:npx vitest run: 266 files, 1,900 tests pass.npx vitest run --config vite.config.hydrate.mjs: 147 files, 779 tests pass.npx vitest run --config vite.config.server.mjs: 228 files pass.test/server/foreign-raw-text-4704.spec.tsxdoes not compile with my local compiler binary, which is older than the fix for<style>and<script>children inside MathML are not escaped in SSR (and static DOM templates), allowing markup injection #4704.Size, from
node size.mjs --no-gate --jsonon this branch and on its base, thennode gate.mjs head.json base.json: every scenario that includes the client runtime is 42 B smaller minified. "page: compiled live server components" is over its cap on the base as well (45,045 B against 44,950 B with my local compiler binary) and stays over.In vercel-labs/ui on rc.21 with its patch removed, I applied this change to the installed
@solidjs/webbuilds and rannpx playwright test specs/leaks.spec.ts --project solid --workers 1 --retries 0against its dev server with performance tracks off: 47 of 47 pass. I have not rerun the built playground with the change applied.🤖 Generated with Claude Code