Solid: remove the @solidjs/web patch and move to 2.0.0-rc.21 (branch ready, waits on solidjs/solid#4836)

Open
  • Is something wrong?
#487 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
15/100
Issue type
Refactor
Clarity
Mostly clear
Activity status
Active
Tech stack
javascript, typescript
Domain
frontend

Research direction

Start with patches/@[email protected] and the patchedDependencies entry, then the leak spec in the Solid playwright project and apps/playground/src/main.ts, which keeps the attribution engine off the leak page. The branch chore/solid-rc-21 holds the version bumps; the work is blocked until a Solid release includes solidjs/solid#4836, and the size budgets in scripts/size.mjs and the packages/solid compiler peer range are still open. Done means the Solid leak spec passes 47 of 47 on a built playground and the budget and changeset questions are settled by a maintainer.

Written by the indexing model from the issue text.

Description

The repo patches @solidjs/web 2.0.0-rc.14 (patches/@[email protected]) so that delegated events share one currentTarget getter. I have a branch that moves Solid to 2.0.0-rc.21 and removes the patch, and two Solid pull requests that fix the cause. I can't open a pull request here (the repo takes them from collaborators only), so this issue carries it.

Branch: https://github.com/brenelz/ui/tree/chore/solid-rc-21 (compare)

Why the patch is still needed on rc.21

Solid's delegated event handler defines a currentTarget getter on each event, a new closure each time. V8 keeps the first of those getters on the shape that all events of that type share, so one event, its target and the page around it stay alive. rc.19 and later delete the getter after dispatch, but that does not remove it from the shape. On rc.21 without the patch, 4 of the 47 Solid leak tests fail on a built playground: combobox, slider and resizable after 200 cycles, and tabs unmounted right after an arrow key.

solidjs/solid#4836 sets currentTarget as a plain value and creates no getter. With that change applied by hand to the installed @solidjs/web, the Solid leak spec passes 47 of 47.

A second leak on the dev server

@solidjs/vite-plugin 3.0.0-next.57 turns Solid's performance tracks on under vite dev, and the attribution engine behind them keeps every scope that re-ran. apps/playground/src/main.ts keeps that engine off the leak page for this reason, and the tracks turn it on there. On the dev server all 47 Solid leak tests fail on rc.21. CI and pnpm test:pr serve a built playground, which has no tracks.

solidjs/solid#4837 (draft) makes the engine release disposed nodes. With both changes applied by hand, the leak spec passes 47 of 47 on the dev server with the tracks on. The plugin also has a performanceTracks: false option that apps/playground/vite.config.ts could set.

What the branch changes
  • Catalog: solid-js and @solidjs/web to 2.0.0-rc.21, @solidjs/vite-plugin to 3.0.0-next.57. @solidjs/compiler in packages/solid to 2.0.0-rc.21.
  • Removes the patch and patchedDependencies.
  • Updates .agents/experts/solid/decisions.md and knowledge.md.

It should wait for a Solid release with solidjs/solid#4836, because the four leak tests fail until then.

Checked on the branch
  • pnpm typecheck, pnpm lint, pnpm build and pnpm format pass. The packages/solid unit tests pass: 51 files, 1,332 tests.
  • PLAYGROUND_BUILD=1 npx playwright test --project solid --project solid-data --project solid-ssr --retries 0: 5,818 pass, 361 are skipped, 17 fail. Four are the leak tests above. The other 13 are five modal specs across the three projects (csp.spec.ts:83, dialog.spec.ts:601, dialog.enhancements.spec.ts:158, dialog.patterns.spec.ts:68 and :240, menu.enhancements.spec.ts:81). They read the scroll lock or inert right after the dialog is visible, and the same specs fail for --project react on my machine, so I take them to be a local timing difference.
  • I have not run the built playground with the Solid fix applied.
Open questions
  • Size budgets. node scripts/size.mjs lists 60 @ui-sdk/solid/* entries over budget, against 2 on main. Each grows 1.4 to 2.2 kB gzip from the rc.21 runtime, for example solid/button 26.21 to 27.69 kB. The branch does not update the budgets.
  • Changeset. The compiler bump changes one published file. dist/tree.js no longer calls claimElement on the tree item's link, because rc.21's spread claims it. The other 65 compiled files are byte-identical, and every import in the compiled output exists in rc.14. The peer range still starts at rc.14, where spread does not claim the element while hydrating. The choices are to keep @solidjs/compiler at rc.14, to add a changeset, or to raise the peer floor.
Dominant language
TypeScript
Stars
4.4k
Forks
122
Avg merge
3h 12m
Merged PRs (30d)
437

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from vercel-labs/ui

All issues in vercel-labs/ui

Similar issues

More TypeScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.