refactor: split the catalog into minimal and basic modules - #24
Merged
brianegan merged 8 commits intoSep 12, 2026
Merged
Conversation
Moves `lib/src/catalog/` into `minimal/` and `basic/` layers, with Basic depending on minimal and nothing pointing back. `jaspr_component.dart` and `catalog_extension.dart` stay shared above both. Every component now declares the default rules for the classes its `build` emits, through a `styles` member on `JasprComponent`, and a catalog derives its bundle from the components it holds. That makes a catalog built with `copyWith` carry the right rules by construction rather than leaving the consumer to assemble a second list by hand. Removes `genuiJasprStyles`. The package ships no surface-level CSS at all: `.a2ui-surface` and `.a2ui-missing` belong to no component, so the example app styles them now. `themeProperties` moves next to `surface.dart`. Closes #23
The tree-shaking fixtures read `styles` but never called `build`, so a Basic-only marker's absence proved nothing about catalog code. Worse, the marker was a bare class name, which `build` emits too, so it could have passed on catalog code alone with style code unmeasured. Each half now has a marker only it can produce, and the fixtures exercise both paths. Also shares one catalog value across the example, so its style bundle and its renderer cannot drift apart.
The demo now exercises every component the package ships rather than the five-component minimum. `appCatalog` stays a single value read by the styles, the renderer and the prompt, so the rules the page ships and the components it can build cannot drift apart. Not made configurable: `Chat` is the `@client` boundary and its parameters must serialize for hydration, so a catalog cannot reach it. An `App` that took one would change the page's styles without changing what the client renders.
Switching the example to Basic left `chat_browser_test.dart` declaring the minimal catalog id, so its canned reply opened a surface against a catalog the conversation did not hold and nothing rendered. It reads `appCatalog.id` now, so it tracks the app rather than pinning a second catalog. The VM suite passed throughout, which is why this went unseen: only the browser suite reaches it, and `example/tool/coverage.sh` aborts there. Also gives the example's style test teeth. It asserted `.a2ui-button`, which both bundles carry, so it could not have caught a regression to minimal. It now checks a Basic-only selector and the two surface classes the app owns. Names the second source of the tree-shaking build marker, fixes the README quickstart to carry one catalog value across both snippets, and restores the dart:/package: blank line in main.server.dart.
… catalogs Reverting `import_only.dart` to its original form took the `.styles` read with it, and that fixture is the only minimal-side one checked against the icon table. So nothing was proving that a minimal program reading its styles keeps the 59 Material paths out, which is exactly the path the derived getter introduced. `minimal_only.dart` now carries that assertion, and adding `IconComponent` to it turns the test red. Renames the fixture from `minimal_styles`, which described half of what it does now that it builds components too, and drops two props no component it builds ever reads. The round-trip test pinned the catalog id twice while the rest of the example reads `appCatalog`, the same drift that broke the browser test. Both read `appCatalog` now. Same for the README, which told you to hold one catalog value and then built a second one inline two sections later.
`JasprComponent.inline` is the documented way to make a one-off component, and it could take an api and a builder but not rules. So a catalog extended the documented way was the one kind whose derived bundle came out wrong, and the README's claim that the bundle stays correct for any catalog was false for exactly that case. It takes `styles` now. Covering the base `styles` default needed a bare subclass: `inline` used to be the only thing reaching it, and once it carried its own field the default went unexecuted. A test component that overrides nothing else reaches it, and that is the real case the default exists for. Also names the three parts in the class doc example, says what the catalog extension does now that it does two things, renames a temp dir that still said "styles", and drops a duplicate catalog declaration in the README.
Giving `JasprComponent.inline` a `styles` parameter fixed the dartdoc's "two halves" phrasing and left the README's copy of it, plus two other places that still described a component as an api and a builder. All four now agree: the prose defining a component, the `DividerComponent` example, which emitted `a2ui-divider` and declared no rule for it sixteen lines above the claim that components declare their own rules, the `inline` signature, and the changelog entry. The example also shows what deriving the bundle buys: `catalog.styles` picks up the divider's rule with no second list to update.
The `JasprComponent` class doc still called a component an API paired with a renderer, with "the Jaspr half" singular, in the paragraph immediately above the three-part example the last commit added. That was the fifth site of a shape I had already claimed was fixed everywhere. Also stops the docs naming a member that does not exist. The derived getter is on the `JasprCatalogComposition` extension over `Catalog<JasprComponent>`, not `Catalog.styles`, which reads as a2ui_core's own class.
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
1 participant
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
Splits the flat
lib/src/catalog/tree intominimal/andbasic/layers, and moves every CSS rule onto the component that emits the classes it targets. A catalog derives its own style bundle from its components, so any catalog is styled correctly by construction, including thecopyWithderivations the README documents. The package now ships no surface-level CSS at all.Changes
lib/src/catalog/splits intominimal/andbasic/, with Basic depending on minimal and nothing pointing back. One barrel, unchanged apart from paths. Reachability does the rest.JasprComponentgainsstyles, overridden by all 18 components next to thebuildthat emits the classes.JasprComponent.inlinetakes them too, so the documented one-off path produces a complete bundle.Catalog<JasprComponent>gains a derivedstylesgetter besidecopyWith. It lives on the extension, not the catalog classes, becausecopyWithreturns a plainCatalogand a member would not survive derivation.genuiJasprStylesis gone, along with the.a2ui-surfaceand.a2ui-missingrules. No component emits those two, so the example app owns them now.themePropertiesmoves next tosurface.dartandlib/src/styles.dartis deleted.BasicJasprCatalog, reading one sharedappCatalogvalue from its styles, its renderer and its prompt.Testing
The change is mechanical where it matters and the suites cover it, so the honest answer is that the lights are green: 264 tests at 100% line coverage, plus the browser suites on both the package and the example.
Two things are worth a reviewer's own eyes, because both are assertions about a compiler and about CSS that a passing test can hide.
The tree-shaking claim is the load-bearing one.
test/icon_tree_shaking_test.dartpairs every absence assertion with a presence fixture, because an absence alone passes whether or not the fixture compiled the code it is looking for. Two separate markers, one only aStyleRulecan emit and one only the component carries, so catalog code and style code are measured independently rather than one covering for the other. Worth checking that pairing is real: delete either presence assertion and the absence beside it stops meaning anything.For the appearance,
dart run jaspr serveinexample/and look at a generated surface. It should be unchanged from main. The rules all moved, so the risk is a selector that landed on the wrong component or got dropped in the move rather than anything subtle.Gates
Gated
8b99bedagainstorigin/main@8710ee6.code-reviewonfable, passed, agent-reportedReviewed against the plan recorded on the issue.
Closes #23