Refactor: Rework slots so each file has one job, and report compile errors as diagnostics - #2420
Merged
Conversation
github-actions
Bot
added
ruby
marcoroth
force-pushed
the
client-refactor
branch
from
August 27, 2026 07:38
a3cfc80 to
7a9f4cd
Compare
dac34ec moved the clause grammar and the attribute vocabulary into parsing.ts and attributes.ts and dropped the re-exports from directives.ts, but eleven linter rules and five language-service modules import those names from @herb-tools/client/directives. Put the re-exports back so both packages typecheck against the client again.
A payload carries both a block's whole output and the slots inside it. Writing the whole output first replaced the interior markers, so the interior values then had nowhere to land.
One round-trip fixture checked the client against the compiler. The table covers every construct parity_test.rb lists, in both slot modes, with the schema and the dependency map, plus the marker grammar, so a change to either side shows up as a fixture diff.
The seeds marker, the part marker, the statics keys and the seeds filter expression were written inline in the visitor and the values compiler. One home for the grammar, and one filter expression for both compilers.
apply() opened a transaction of its own, so every payload was recorded and the token was read by nothing. Wrapping apply in transaction() records it; on its own it does not. The retain option had one value.
Three WeakMaps held facts about a slot on the side, which made addressing a slot a walk over every collection's items and kept the state layer from asking a slot where it lives.
claimed(), currentValue(), the markup operation, Region.start/end, the two-answer row template, the data-herb-statics container no compiler emits, and the payload scope of a send.
Three marker handlers each decided on their own when to trust the parse stacks and when to fall back to document geometry, and had drifted: a slot rescanned inside an item never found the item. A scan now seeds its stacks once, from the caller's context or one locate(), markers it has already indexed replay onto the stacks, and the handlers read nothing else. locate() and placements() are public, and the state layer uses them instead of its own copies of the geometry.
Twenty-seven switches on anchor.kind expressed the same five operations: the range, the host element, the element that takes attributes, the current markup or text, and whether it is still on the page.
The compiler parks statics; the client called the same fragments skeletons, a word wanted for loading placeholders. parked() and parkedKeys() replace skeletonFor() and skeletonKeys().
A block like <% foo do %> received a slot and a marker pair, but the values compiler never produced a value for it and the client never read its type. Its closing marker was also the last expression of any lambda or helper block that wrapped it, which returned the buffer instead of the block's value.
A collection that rendered rows shipped a blanked copy of a row it already contained. The client builds an added row from a live one, so the copy was only needed when no row rendered.
_herb_covered_branches was a local reset on every render, so a partial rendered fifty times parked the same branch fifty times. It is now a view ivar keyed by the template identifier, initialized once per response and marked as each branch's statics are emitted.
…collection JavaScript sorts integer-like object keys, so a payload lost the order the server rendered a collection in. An unkeyed collection emits no item markers, so the items it reported could never be applied; its interior is now captured and discarded rather than landing in the scope around it. Region-scoped seeded states ship with the values, the way an item's already do.
Under an escaping engine the marker's key went through the escape function while the payload's went through to_s, so a key holding any of &<>"' named a row no payload could reach. The marker now appends the key to the buffer directly.
Nodes added into two different items of one collection share a parent, so seeding once for the group indexed the second node into the first node's item. A root re-seeds from its own placement unless the previous root left a marker open.
A slot inside a collection nested in another collection was addressed as if its collection sat at the region, so the address resolved to nothing and an inverse was dropped. The same lookup now finds a nested collection when its items are scanned.
An element inside a collection nested in another collection resolved only against the innermost item, so a state the outer collection declares was unreachable from the inner one.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
marcoroth
changed the title
marcoroth
force-pushed
the
client-refactor
branch
from
August 27, 2026 07:44
7a9f4cd to
23cd910
Compare
🌿 Interactive Playground and Documentation PreviewA preview deployment has been built for this pull request. Try out the changes live in the interactive playground: 🌱 Grown from commit ✅ Preview deployment has been cleaned up. |
@herb-tools/client
@herb-tools/core
@herb-tools/dev-tools
@herb-tools/formatter
@herb-tools/language-server
@herb-tools/linter
commit: |
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
2 participants
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.
This pull request reworks the slots client and the slots compiler so that each file has one job and names it, and changes what the compiler does about a template it cannot compile.
The client had grown into a handful of files that each did several things, with private
#fieldsthat made the seams hard to see. The compiler had the same problem in reverse, with a template's identity spread across the payload, the markers and the dependency map. Neither had a good answer for a template that will not compile, which raised on the first thing it disliked and told the developer only a sentence.The client
Slotswas carrying the region index, the collections, the journal and the scanner. Each of those is now its own file with a delegate, in the shapeElementObserveralready used, so a caller says what it wants and the holder says what happened. The same split runs through the state layer, which now separates what it holds from what it asks the server for, and throughActions, which reads an element's actions once instead of on every event.Everything is addressed by name now.
SlotIndexbecameRegionIndex,HerbRuntimebecameRuntime, and the files sit inslots/,state/,actions/,markup/,grammar/,outbox/andshared/according to what they do.The manifest
A template used to say what it is in three places. The manifest is now the only one, carrying the file, the version, the names, the parts and the states, and both deliveries hand a page the same shape so a client reads them the same way. A project can gather its manifests ahead of rendering through
Herb::Engine::Slots::Manifest::Collector, and a response delivers them once however many times a partial renders.Compile errors
The slot compiler raised
Herb::Engine::CompilationErrorfrom 43 places.CompilationErrorwas a bareStandardError, so a developer got a sentence and a backtrace pointing into the compiler. A template with three states gave no clue which one was wrong.The compiler now tells the visitor, which records a diagnostic and keeps walking, so one template reports everything wrong with it at once and each finding carries the line and column it belongs to. The messages were rewritten to match the linter's voice, saying what is wrong and then what to do:
A visitor is fatal by default and still raises once the walk is over, which is what the CLI and the tests expect. A visitor that is not fatal lets the template compile without markers or a payload, so the page renders on the server and the findings reach the browser through
Herb::Engine::Report::Middleware.A refused template keeps the region it opened and drops only the manifest. A region carrying no manifest is how a page says the compiler refused it, so the client leaves everything inside one alone instead of complaining about every control the author wrote. Before that, one compile error became sixteen findings in the browser, fifteen of them false.