Convert StyleResolutionTrait into a StyleResolver collaborator (#242) - #1318
Merged
Conversation
At 2,822 lines this was the largest single-consumer trait mixed into HtmlTransformer and the largest single contributor to that class's object scope. Despite its size it was the most self-contained of the remaining traits: 15 transformer operations and one cache, fewer external dependencies than traits a third its size. All 292 call sites across the transformer and six sibling traits were migrated, so the trait is deleted rather than hollowed out. DomHelpersTrait gains an onSourceMarkupMutated() hook. Link canonicalization there invalidates a selector-match cache that only the transformer owns, but the trait is shared by four classes; the hook defaults to a no-op and HtmlTransformer overrides it.
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.
Slice 6 for #242, workstream 1. Follows #1310, #1312, #1315, #1316, #1317.
This is the big one.
StyleResolutionTraitwas 2,822 lines — the largest single-consumer trait mixed intoHtmlTransformer, and the single largest contributor to that class's object scope.The surprise
I expected this to be the hardest remaining trait. Measured, it was the easiest:
StyleResolutionTraitFormDispatchTraitThe biggest trait had half the external coupling of one two-thirds its size. Size and coupling were inversely related here, which is worth knowing before picking the next one.
What changed
StyleResolutionTrait→StyleResolver, behind a 15-operationStyleResolutionContext. Per-transform state (author styles, source styles, layout geometry, presentation cache, transformation evidence) resolves through closures, because the resolver is built once with the transformer but must see the state of the transform currently running.StyleAttributeMapperandHighValueStyleBoundaryPolicystay self-owned lazy singletons — they were already the resolver's own, not the transformer's.292 call sites migrated, trait deleted, no shims:
HtmlTransformerSvgMaterializationTraitElementConversionTraitNavigationStyleProjectionTraitFormDispatchTraitNavigationToggleSuppressionTraitDomHelpersTraitA trap this surfaced
DomHelpersTraitis shared by four classes, only one of which is the transformer. ItscanonicalizeLinkUrls()invalidated a selector-match cache that only the transformer owns — and it is reached transitively frominnerHtml(),innerHtmlPreservingWhitespace(), andouterHtml(), which all four consumers call.A naive migration would have left the shared trait referencing a collaborator that
SubtreeClassifier,FallbackEmitter, andSemanticParityReporterdo not have. It would have worked in the corpus run and blown up on whichever path first canonicalized a link from one of those three.Fixed with an
onSourceMarkupMutated()hook on the trait that defaults to a no-op;HtmlTransformeroverrides it to invalidate the cache. The shared trait no longer knows about a collaborator three of its consumers lack.Behavior preservation
The corpus check earned its keep twice here: it caught a namespace collision on
HtmlTransformerAnalysisCache(resolved into...\HtmlToBlocks\Style\instead of...\HtmlToBlocks\) as 385/385TypeErrors the moment it was introduced.composer testexit 0.Test updates
Three unit tests reflected on style methods via
HtmlTransformer. They now reachStyleResolverthrough the transformer's collaborator.presentationCacheKeystayed private and is reached by reflection on the resolver rather than widening its API for a test.Impact
HtmlTransformer.php−2,773 lines off the object scope in one slice — more than the previous five slices combined (−386).
Cumulative since the audit: effective object scope 22,026 → 18,867 (−14%), three traits eliminated,
convertElement()441 → 301.Scope
Five single-consumer traits remain, 5,542 lines:
FormDispatchTrait(1,865),NavigationStyleProjectionTrait(1,319),SvgMaterializationTrait(983),NavigationToggleSuppressionTrait(781),ElementConversionTrait(594).Given what this slice showed about size vs coupling, the next one should be chosen by measuring external dependencies rather than line count.
AI assistance disclosure: implemented and drafted by Claude Sonnet 4.6 running in Claude Code, operated by @chubes4. The AI measured trait coupling to select the target, captured the pre-change corpus baseline, converted the trait, migrated all 292 call sites, identified and fixed the shared-trait hazard in
DomHelpersTrait, and verified byte-identicalserializedBlocksand fallback counts across 385 fixtures. Reviewed by a human before opening.