Repository navigation
fix(signals): compare symbol keys in UNSTABLE_MEMO_OUTPUT - #3775
Merged
Merged
Conversation
🦋 Changeset detectedLatest commit: 2b3079a The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 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 |
Size (brotli, eager entry chunk)
Bundled with Rolldown (what Vite ships), brotli q11, decimal KB. Caps in |
Coverage Report for CI Build 37281019340Coverage remained the same at 75.991%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
The unstable-output check compared plain objects by Object.keys, which
skips symbol keys. A memo that returns a fresh symbol-keyed box each run
looked like a run of empty objects, so it warned as new-but-equivalent
although every box held a different value. web's dynamic does this for
an in-flight promise ({ [FLIGHT]: promise }), so every refetch through
dynamic counted toward the warning.
shallowEquivalent now compares Reflect.ownKeys. Boxes whose symbol-keyed
values are identical still warn.
Co-authored-by: Claude via Cursor <noreply@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
ryansolid
force-pushed
the
fix/unstable-memo-symbol-keys
branch
from
October 5, 2026 08:00
eec54bf to
2b3079a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
UNSTABLE_MEMO_OUTPUTcompared plain objects byObject.keys, which skips symbol keys. A memo that returns a fresh symbol-keyed box each run therefore looked like a run of empty objects and was reported as "new-but-equivalent", even though every box held a different value.dynamicin@solidjs/webreturns{ [FLIGHT]: promise }from its factory memo while a result is in flight, so every refetch throughdynamiccounted toward the warning. Found in the server todos example: toggling rows warned on<Todos> › computed, which is thedynamic(() => getTodoList(...))factory. Each refetch boxes a new promise, so it was a false positive.shallowEquivalentnow comparesReflect.ownKeys, so symbol keys are included. Objects whose symbol-keyed values are identical still warn.Public API changes
UNSTABLE_MEMO_OUTPUTnow counts symbol-keyed (and non-enumerable) own properties when deciding whether two plain objects are equivalent. Plain objects that differ only in a symbol-keyed value no longer warn. No exports, options or diagnostic codes change.Verification
packages/signals/tests/attribution-unstable-memo.test.ts: one memo whose symbol-keyed value changes every run stays quiet, and one whose symbol-keyed value is always the same object still warns. It fails before the fix.@solidjs/signalssuite passes, and the source type-check (tsconfig.build.json) is clean.<Todos> › computedwarning no longer appears. The remaininguseSubmissionswarnings there are fixed separately by fix: useSubmissions keeps the same list while its submissions are unchanged solid-router#653.