Repository navigation
Conversation
…ollections Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Owner
|
Thanks! I will probably leave reviewing this for later, as your other reported correctness & behaviour issues take priority. |
Keep the one-pass path for default keys while custom codecs continue to use their overridden decode and encode methods. Co-Authored-By: Codex GPT-6 Astra <codex@openai.com>
Owner
|
@nguyenyou Is the second commit a fix for something in the first commit, or for a pre-existing issue in Laminar? |
Contributor
Author
|
@raquo The second commit fixes an issue introduced by the first. The optimization skipped custom codec behavior; the fix preserves it while keeping the speedup for default codecs. |
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.
Today
Every
cls <-- signalemission ends inupdateCompositeValue, which rebuilds the attribute string like this:getAttribute → decode (split → filter → wrap → List) → filterNot → ++ → mkString → setAttribute
That's ~5 short-lived collections per update, usually for a one- or two-word string.
This PR
Default composite keys produce the same result in one pass:
DefaultCompositeCodec.encodeUpdated(domValue, removeItems, addItems)walks the split DOM value once, skips removed items, appends added ones, and builds the string directly. It equalsencode(decode(domValue).filterNot(removeItems.contains) ++ addItems)for the default codec.CompositeCodec.encodeUpdatedcalls its overridabledecodeandencodemethods, preserving the behavior of custom codecs.CompositeCodec.normalize: fast path when there's no separator ("active"→List("active"), no split); otherwise builds the List directly instead ofjsSplit → ew.filter → asScalaJs.toList.updateCompositeValuereuseskeyItemsWithReasoninstead of looking up_compositeValuestwice.No behaviour change: the bookkeeping, the DOM read (kept for third-party classes), and the exact attribute string written are all the same as before.
Numbers
n elements with
cls <-- sharedSignal, toggling the signal. fullLinkJS, Chrome 152, M3 Max, runs interleaved A/B/A/B:These measurements predate the custom-codec compatibility follow-up; default keys still use the same one-pass implementation. Control, the same binding with
classset via a plainhtmlAttr: unchanged (~200 µs @ 1000 rows). Soclsis still ~2× a plain attribute – mostly the DOM read, which I kept on purpose.Testing
airstream_sjs1_2.13:18.0.0-M5-SNAPSHOTis unavailable.normalizeand the default codec'sencodeUpdatedagainst the original code on 20k random inputs (extra / leading / trailing separators, duplicates, empty / unset values," "and","separators): identical output. I didn't add this as a test because of the "no speculative tests" note innotes/Testing.md– happy to add it if you'd like.🤖 Generated with Claude Code