Skip to content

fix(web): track in-place class mutations - #3191

Merged
ryansolid merged 2 commits into
solidjs:nextfrom
nickshiro:fix/3188-in-place-class-mutation
Sep 2, 2026
Merged

fix(web): track in-place class mutations#3191
ryansolid merged 2 commits into
solidjs:nextfrom
nickshiro:fix/3188-in-place-class-mutation

Conversation

@nickshiro

Copy link
Copy Markdown

Summary

fix #3188

Object-valued class bindings did not update correctly when the same object was mutated in place and passed through a signal using { equals: false }.

The previous value retained the same object reference, so the class diff compared the mutated object against itself and could no longer determine which classes had previously been applied.

This change stores an independent snapshot of the classes applied by className(). It allows in-place mutations to be diffed correctly while preserving classes added by external code.

The snapshot is also initialized during hydration so subsequent in-place updates behave consistently on claimed DOM nodes.

How did you test this change?

Added regression tests covering:

  • In-place mutation of an object-valued class binding
  • Preservation of classes added by external code
  • Transitions between object, string, and null class values
  • In-place mutation after hydration

Ran:

pnpm --filter @solidjs/web build
pnpm --filter @solidjs/web types
cd packages/web
../../node_modules/.bin/vitest run
../../node_modules/.bin/vitest run --config vite.config.server.mjs
../../node_modules/.bin/vitest run --config vite.config.hydrate.mjs
../../node_modules/.bin/tsc --project tsconfig.test.json
../../node_modules/.bin/prettier --check \
  src/client.ts \
  test/class.spec.tsx \
  test/hydration/class.spec.tsx \
  ../../.changeset/track-applied-classes.md

Results:

  • Client: 701 tests passed
  • Server: 536 tests passed, 2 skipped
  • Hydration: 152 tests passed
  • TypeScript and Prettier checks passed

@changeset-bot

changeset-bot Bot commented Sep 1, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: e90f776

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

This PR includes changesets to release 11 packages
Name Type
@solidjs/web Patch
@solidjs/babel-plugin Patch
@solidjs/element Patch
@solidjs/h Patch
@solidjs/html Patch
test-integration Patch
@solidjs/compiler Patch
@solidjs/diagnostics Patch
@solidjs/signals Patch
solid-js Patch
@solidjs/universal 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

@codspeed-hq

codspeed-hq Bot commented Sep 1, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 136 untouched benchmarks
⏩ 132 skipped benchmarks1


Comparing nickshiro:fix/3188-in-place-class-mutation (e90f776) with next (8f59ebe)2

Open in CodSpeed

Footnotes

  1. 132 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

  2. No successful run was found on next (e90f776) during the generation of this report, so 8f59ebe was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@ryansolid

Copy link
Copy Markdown
Member

I independently validated the implementation and tests: the per-element applied-class snapshot is the right fix, including hydration seeding and preservation of externally added classes. This is good to land.

next has since landed #3189 in the same className function, so the PR now conflicts. Could you rebase onto current next? The semantic resolution is mechanical: keep #3189's number-to-string normalization at the top of className, retain your _$classes snapshot/object-diff path, and keep #3189's standalone-boolean filtering in flattenClassList below. Once rebased, the stale size failure should also rerun against the current baseline (the old failure was broad signals/app budget drift, not attributable to this web-only change).

@ryansolid
ryansolid force-pushed the fix/3188-in-place-class-mutation branch from f990181 to 097025f Compare September 2, 2026 04:20
@nickshiro

Copy link
Copy Markdown
Author

@ryansolid, thanks for your review. I went to bed yesterday and wasn't able to reply right away.

@nickshiro
nickshiro force-pushed the fix/3188-in-place-class-mutation branch from 097025f to 23ca619 Compare September 2, 2026 04:56
Document and ratchet the two app scenarios that retain the in-place class mutation fix.

Co-authored-by: Cursor <cursoragent@cursor.com>
@ryansolid
ryansolid force-pushed the fix/3188-in-place-class-mutation branch from 23ca619 to e90f776 Compare September 2, 2026 05:11
@ryansolid
ryansolid merged commit 77ef69f into solidjs:next Sep 2, 2026
6 checks passed
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