Repository navigation
refactor: complete the repository-wide cleanup and lifecycle ownership - #16
Merged
Merged
Conversation
Records the repository-wide audit (scored dimensions, hotspots, dependency/icon/CSS inventories, regression risks) and the execution plan derived from it, following documents/REFACTORING.md sections 23-24.
ChatBadges returned null before creating its memo, so the reactive owner depended on the mount-time value of hideAllBadges. Existing message rows never reacted when the setting changed; only newly mounted rows picked it up. The check now lives inside the memo.
Replaces two overlapping icon sources with one: the Hugeicons icon font (964 KB woff2 plus a 420 KB stylesheet, used through stringly-typed hgi-* classes) and lucide-solid (5 deep-subpath imports). Adds a typed Icon primitive over @hugeicons/core-free-icons (MIT, zero dependencies, per-icon subpath exports) and a PlatformGlyph module that owns the monochrome brand glyphs previously duplicated inline in setup.tsx and TwitchChannelField.tsx. Custom event, reply, role and sparkline graphics stay inline because they are bespoke visuals rather than UI icons. Removes the font assets, the index.html stylesheet link and the now-dead .hgi-stroke rule. Production dist shrinks by about 1.35 MB.
…llision root.css declared a second, parallel token set alongside the SolidCN tokens in app.css. Six of its tokens were unreferenced, --panel and --text-secondary were used only by the unused .bg-panel and .text-secondary utility classes, and --border was declared in both files with incompatible values so that the app.css definition silently won. --text stays because the overlay renders on a transparent OBS background and must not inherit the themed foreground token, which is near-black in the light theme. Verified by computed style: body color is still rgb(255,255,255) and --border resolves from app.css alone.
Deletes nine modules that nothing imported from either src or tests: the chat parsing/formatting leftovers sanitize, urlParser, markdownParser, emojiRenderer, actionMessages, userNoticeParser and chatUtils, plus the two barrels utils/chat/index.ts and utils/ui/index.ts. The utils/chat barrel existed only to re-export ChatConfig through a second hop, so the 15 files that used it now import the type from its real home in config/chatUrlParams. YouTubeChatService and types/userNotice.ts were superseded by ExternalChatService and replyThread respectively and had no remaining references. RteCosmeticsService.clear() is deliberately kept: the audit calls for that cleanup to be wired into its owning runtime rather than deleted, which is a separate behaviour change.
The Twitch-bot, YouTube-bot, Kick-bot and viewer-allowlist rows each repeated the same borderless input markup and class string. They now share components/setup/SetupChipInput, a feature-level wrapper rather than the generic Input primitive, because the field sits inside an existing bordered chip container and must stay borderless and transparent. Moves ColorPickerField under components/setup/ so feature components live with their feature. The remaining raw controls in setup.tsx are bespoke single-purpose elements with their own CSS contracts; pushing them through Button would inject conflicting base styles. Verified in a real browser: all four rows still commit a chip on Enter and clear the draft, with no console errors beyond an expected 404 from kick.com for a probe username.
renderMessageContent.ts declared codePointToCodeUnit twice with identical bodies, and escapeAttr byte-identically to its copy in emoteModifiers/html.ts. escapeAttr now has one definition, exported from the module that already owns chat HTML-string assembly, and codePointToCodeUnit is hoisted to module scope with a note on why Twitch ranges need the conversion. Behaviour preserving: tests/renderMessageContent.test.ts and tests/messageTokenSnapshot.test.ts pass unchanged, the overlay renders identically and the chat chunk is marginally smaller.
The route declared the storage key registry plus its own guarded read/write helpers, while config/setupTemplates carried a second, differently shaped copy of the same availability and try/catch logic. Both now go through services/storage/setupStorage. The adapter stays dependency-free so the config -> services edge it introduces cannot form a cycle; the alternative, relocating template persistence into services, would have been a larger move for the same result. The injectable StorageLike parameter is preserved because tests/setupTemplates.test.ts passes an in-memory store. Verified in a browser: a typed channel is persisted, survives a reload, reaches the generated overlay URL, and removing it deletes the key rather than storing an empty string.
The route declared its own Twitch and Kick bot-profile fetchers, and TwitchChannelField carried a byte-identical fetchJsonWithTimeout plus a second copy of the GQL endpoint and client id. Both now share services/network/fetchJsonWithTimeout and the constants in config/twitch. normalizeBotLogin keeps stripping a leading @ even though config/chatUrlParams parseBotNames does not, because users paste handles with the sigil while URL values never carry one; merging the two would change URL parsing. Verified in a browser: the Twitch lookup resolves 35 display names and 33 avatars for the default bot list, the Kick lookup resolves its entries, and an unknown login degrades to a fallback chip without a console error.
The route owned the Chromium-only browser sniff, the raw per-face font records and the enumeration try/catch, so the Local Font Access API surface was spread through presentation code. services/setup/localFonts now returns a typed result and the route only maps it onto its status signal. Verified in a browser: the real API yields the empty status without a console error in headless Chromium, and an injected fake reports two families with grouped, sorted styles and drops a whitespace-only family.
The coercion helpers were rebuilt inside the component on every render and one pair of them, toIntOrFalse and toSecondsOrFalse, was byte-identical. They now live in config/formValues at module scope, which is also where the form-side counterpart of the chatUrlParams ParamKind vocabulary belongs. chatUrlParams parses and serializes its intOrFalse and secondsOrFalse kinds through identical code, so the two helpers collapsed into toPositiveIntOrFalse; tests/formValues.test.ts pins that premise and characterizes the form -> URL -> config round trip.
The route owned the whole form-to-config projection and the overlay link builder, which meant the URL contract could only be exercised by driving the page. config/setupConfig now takes an explicit SetupFormState, so the projection is pure and the route only collects signals into it. buildOverlayUrl gained an optional baseUrl so the contract is assertable without a document; the preview still drops ms through the same option it used before. tests/setupConfig.test.ts pins default projection, de-channelling, clamping, malformed input, extra-parameter precedence and a full export -> parse round trip. Verified in a browser: channel, size and line-height each change the generated link and survive a reload, and the embedded preview renders demo messages with ms absent and no console errors.
The route owned the preview iframe protocol directly: the config postMessage, the session-key gate, the debounce timer, the direct src swap and its cancellation. features/setup/previewSync now owns that protocol while the route keeps only the reactive effects that read signals, and the navigation URL is built lazily so an unchanged session costs nothing. The module imports the preview message builder from its own module rather than the preview barrel, which keeps the setup feature from pulling in the emote and style modules and lets the synchronizer run under a minimal window shim. Verified by tests/previewSync.test.ts and in a browser: a committed channel change reloads the frame, a config-only change does not, and the overlay still re-renders from the pushed config.
The route inlined the document style snapshot, the lock and its restore, and the reduced-motion subscription with a manual initial read. features/setup/setupDocument now owns both: lockSetupDocument returns its own release function, and watchReducedMotion reports the current preference immediately so the caller needs no separate read. Verified in a browser: the lock applies on setup, a directly loaded /status does not touch those styles, a client-side route change releases the lock, and a reduced-motion context has the demo paused on mount.
The route owned the searchable-element selector, the section lookup, the highlight class bookkeeping, the counter format and the reveal scroll, while utils/setupSearch held the pure matcher. features/setup/settingsSearch now owns the DOM half and the route keeps only the signals and memos, passing its own scrollToSection in because the workspace scroll state belongs to it. Verified in a browser: a query matching 8 rows across 4 sections steps the counter forward and back and clearing removes every highlight and nav marker.
renderUserChip and ffzBadgeMergeBlock were closures returning JSX inside the route. They are now components/setup/UserChip and components/setup/FfzBadgeMergeBlock, and the badge preview asset URL moved with the block that uses it. UserChip takes the profile map as a getter rather than one resolved profile, because bot profiles arrive after the chip renders; roleBadgeMergeOptions stays in the route because it is the signal wiring. Verified in a browser: 29 bot chips render with 29 avatars and a resolved display name, removing one leaves 28, and the three role pills toggle their pressed state.
… owner liveRuntime.destroy now releases everything its initialize installed: the generated overlay stylesheets, the shared rte proxy flag, and the badge and emote caches, which are keyed by channel and previously outlived the runtime that filled them. badgeService.reset restores a snapshot taken at construction so the built-in fallback badges survive it, and sevenTVCosmeticsService.clearAllCaches is now reached from chatPresentationService.cleanup. Verifying that exposed two leaks from modules that appended a style element unconditionally: two #chat-animations elements existed, so removing one by id left the other, and #chat-layout was never removed at all. The three injectors now reuse an existing element, the layout and fade managers gained cleanup, and OverlayStyleManager.cleanup removes every element matching an id. A MutationObserver trace across teardown shows all five stylesheets removed and none left behind. tests/serviceReset.test.ts pins the emote and badge reset behaviour, including that the built-in fallbacks survive a reset.
…rity ChatMessage had two clearEntryAnimation closures that differed only in whether they removed the measured flow shift property, and removing a property that was never set is a no-op, so one closure now serves both branches. badgePriority moved next to ChatBadges, its only importer, with git mv. Verified in a browser by timing each row from the entry class appearing to it disappearing: flow entered 11 rows with a 495ms maximum lifetime and fade 10 rows with 335ms, and neither mode left a row holding the class or the shift property past the animation window. The layout manager half of the same step is recorded as blocked by layering: chatPresentationService owns it, and moving it under features/chat-overlay would make a service import a feature.
The four chatStyles generators emitted rules whose shape followed the size preset, and both the live overlay and the preview appended them to the document head. getOverlayStyleVariables now returns a property map the way getChatEventStyleVariables does, chat.css owns every rule, and the two runtimes write the same properties instead of each generating their own stylesheet - which also removes the preview's duplicate copy of that logic. SIZE_CONFIGS stays as the single source of truth because renderMessageContent reads it for emote geometry; it now publishes properties rather than rules. The emote scale is folded into the published emote and emoji sizes, since a stylesheet cannot multiply a preset by a runtime value. The boolean variants became :root attributes, and the gigantified width is published under a --chat- name because it is no longer scoped to one element. Verified by diffing computed styles for 20 configurations and 14 selectors before and after. The only differences are inherited gigantified-width readings with none on the element that consumes it, readings of an artificial emoji probe whose real counterpart sets the same size inline, and one container height inside the measured noise floor of two identical runs.
utils/ui/animationUtils held the animation modes, the speed bounds and the pure mappings next to functions that read and write the document, so the URL contract and the setup form depended on a DOM-touching module. The vocabulary moved to config/chatAnimation with no DOM access, and the stylesheet generation moved to services/chat/runtime/animationStyles, next to the overlay runtime that consumes it. Verified in a browser: flow and fade still animate and clean up with no rows stuck and no console error; the module test moved to tests/chatAnimation.test.ts and imports each half from its new home.
renderMessageWithEmotes imported networkClient directly, so the renderer owned network policy and could not run without infrastructure. It now takes a RenderMessageOptions object with a resolveUrl callback and imports only the NetworkRoute type, while ChatText passes the real resolver from the component layer. Two tests cover the seam: every asset url goes through the injected resolver, and a message with nothing to resolve calls it zero times. The existing suite passes the real resolver so its RTE-rewrite coverage is preserved. Verified in a browser: the dev fixture renders 4 emotes and 2 emoji with resolved CDN urls and 24 messages, with no console error.
The appearance, styling, behaviour and toggle descriptor arrays were declared inline in the route, so the route owned both the section contents and the form wiring. They now live in components/setup/sections as factories that take one source object and destructure it up front, which keeps every row body byte-identical to what the route declared and makes the extraction unable to silently change a label or a control. setup.tsx goes from 2779 lines at the start of the phase to 1909. Verified in a browser: each section renders the rows its array declares, toggling a switch changes the generated overlay url in every section that has one, and the settings search still reports 8 matches across the extracted rows with no console error.
The preview pane's controls were inline JSX whose selection thumbs were measured by route-level refs, signals and a resize observer. components/setup/PreviewControls now owns the markup and that measurement together, because the refs exist only to position the thumbs and splitting them across a fifteen-prop boundary would have spread the coordination instead of reducing it. setup.tsx goes from 2779 lines at the start of the phase to 1686. Verified in a browser: four stage swatches with the pressed state tracking the click, the custom colour picker appearing only for the custom backdrop, both thumbs measured at height 36px with translateY 40px and 0, and the pause control toggling its pressed state, with no console error.
The ui cache-boundary group still listed lucide-solid after the dependency was removed, and did not list the icon package that replaced it, so the icons were being bundled into the entry chunk instead of the group the comment describes.
Rewriting the overlay style manager to publish custom properties dropped the stylesheet teardown it used to perform, so the animation stylesheet it injects outlived the runtime. clearOverlayStyles now removes it through clearAnimationStyles, and tests/overlayStyleManager.test.ts guards both the removal and the no-accumulation case. Found by a browser lifecycle check: the running overlay now holds only the animation and layout stylesheets, which are generated by design, and leaving the route removes both and clears every published property.
Walks the section 25 checklist item by item with the evidence each item rests on, and records the closing state of the phases.
The custom wrapper traversed the icon payload, kebab-cased attributes and built the svg tags. The vendor now ships @hugeicons/solid-js, so that logic has one owner again and the local Icon keeps only the project convention: currentColor, 1em default size and decorative accessibility. The payload keeps its own stroke width because forcing one at the root would stroke a fill-only icon. Verified by diffing every svg on the setup page before and after: 65/65 identical geometry, font size, stroke width and colour, no page errors. The only attribute difference is the vendor's explicit color=currentColor on the root, which inherits to the same value. Build: ui chunk +2.77 kB, setup chunk -0.50 kB.
The detailed rules lived only in documents/REFACTORING.md, which has to be discovered manually. The root file now carries the rules an automated change can regress: layer direction, lifecycle ownership, primitive and icon boundaries, the styling and custom-property contract, accessibility and OBS constraints, public contracts, dependency and test policy. It also requires repository-wide refactor work to read both documents/REFACTORING.md and the new completion supplement, which is committed here as the plan input for that work.
The settings search imported SetupSectionId from a presentation component, which is the wrong direction for a shared contract. The identity now lives in features/setup/model/setupSections.ts, so the route, the workspace layout and the search all import the same type without importing each other; the icon and translation metadata stays with the presentation that renders it. Two pure helpers sat in JSX modules that now import the client-only vendor icon renderer, which made their tests unloadable without a DOM: - the Twitch/Kick channel search moved from components/setup/TwitchChannelField.tsx to services/setup/channelSearch.ts; - resolveSetupText and SetupText moved from SetupLayout.tsx to components/setup/setupText.ts. The move also collapsed a real duplication: normalizeLogin, the avatar fallback letter and the login-list merge existed twice, in TwitchChannelField.tsx and botProfiles.ts. They now have one owner in services/setup/logins.ts. Verified: an import-boundary sweep finds no features-to-components, features-to-routes or lower-layer-to-upper-layer edge left; typecheck, lint and 375 tests pass; in the browser the Twitch search resolves a login with its avatar and commits it to the overlay URL (c=linaryx) and the Kick search returns its suggestions, with no console error.
The constructor builds the live or preview runtime and the predictions controller, but destroy() returned immediately when start() had never run, so construction and teardown disagreed about ownership. destroy() now releases what the constructor built, is idempotent, and is terminal: a destroyed application cannot be started again, which is also what makes it safe while start() is still awaiting initialize(). tests/chatOverlayApplication.test.ts characterizes construct-destroy, double destroy, double start, start after destroy and destroy during a pending initialization.
Resetting a singleton dropped what it held but not what was already in flight: a response that arrived after the owning runtime was destroyed still committed into the store the next runtime had just cleaned. The same held for the 7TV visibility toggle, where a response loaded under the previous filter could land after the cache was cleared. Each affected service now carries a generation that reset (or, for the cosmetics caches, clearAllCaches) bumps. A load captures it on entry and skips its commit when the store moved on. Covered: the emote global and channel loaders, the badge Twitch, third-party, GQL-set and per-user loaders, the 7TV cosmetics catalog, channel and user loads, and the feature integration, which could mark itself initialized after being destroyed and then block the next runtime. tests/assetTeardownRace.test.ts holds each response open, resets, and only then resolves it: all seven tests fail against the previous code.
The preview runtime enabled the shared RTE proxy flag and filled the shared emote, badge and mention stores, but its teardown released neither, so a document that had shown a preview kept the demo's state and proxy routing after the runtime was gone. destroy() now clears the flag and the stores, and only for a runtime that actually claimed them: holdsSharedState is set by initialize()/updateConfig(), so destroying a never-initialized runtime does not disarm another owner. A destroyed runtime also ignores further configuration instead of re-injecting styles it just cleaned up. The ownership question is recorded in the code: concurrency is not possible by design, because a document hosts exactly one chat runtime, which is why a process-wide boolean is sufficient here rather than a lease or reference count. tests/previewRuntimeTeardown.test.ts covers the release, the never-claimed case and the post-destroy update. Verified in the browser that the setup preview still commits a channel, reloads the frame, picks up a config-only font change (20px to 48px) and reports no page error.
The helper wrote a literal #09090b onto html and body, which the app-wide OBS contract already overrides: app.css declares background: transparent !important for html, body and #root, and the workspace paints its own surface on .setup-root, which covers the viewport. Measured before the change: the inline colour resolved to rgba(0, 0, 0, 0) while .setup-root painted rgb(9, 9, 11). The lock now sets data-setup-document on the root and the setup stylesheet owns what that means: no document scrolling, with the height the columns rely on. The release restores exactly the attribute state it found. Verified by a DOM-level comparison rather than by screenshot: the page is not pixel-stable here (two shots of an unchanged state differ, even in a region with no animation), so a pixel hash could not prove anything. Comparing every element's rect plus the lock-relevant computed styles and every scroll height between the two implementations reports zero changes, and in the browser the lock is applied on setup, removed when navigating in-app to /status, and re-applied on a setup route, with no page error.
layoutUtils and fadeUtils are not general helpers: they publish stylesheets written against #chat_container, .chat_line and .message-fade-out, and ChatPresentationService is their only consumer. They now live in services/chat/runtime/, which already holds the chat runtime's other infrastructure, so the dependency direction is unchanged and the files sit in the layer that owns them. That also resolves the two-classes-one-name problem: the second LayoutManager in utils/ui/layoutManager.ts is deleted. Its only caller was chatFeatureIntegration.setOptions, wiring reverseLineOrder and singleChatter into it, and it was provably a no-op: setContainer was never called, so applyLayout returned before touching anything, and its shouldShowMessage, getContainerClasses, getLayoutCSS and injectCSS had no callers. The singleChatter behaviour that actually runs is ChatPresentationService's own. Verified in the browser: the preview overlay publishes #chat-layout, applies layout-vertical/layout-horizontal/layout-reverse, renders 9 messages, keeps exactly one #chat-animations stylesheet and reports no page error, in all three layout modes.
The 7TV paint stylesheet was created on demand and never removed: its owner generated rules from provider data and left the element in the document after the runtime that filled it was torn down, and the cached sheet reference would have kept writing into that detached element. clearAllCaches() still clears only caches; disposeStylesheet() removes the element and drops the reference, and ChatPresentationService.cleanup() calls both. tests/assetTeardownRace.test.ts asserts the element is gone after teardown and that the next paint render rebuilds it.
The route created the synchronizer with a dispose() that cancels its pending navigation timer, but only the reduced-motion watch and the document lock were released. The timer could still fire after the setup route was left and assign src on the detached iframe.
The manager contract allows apply after cleanup; the stylesheet, the published properties and the boolean attributes all have to come back.
Both comments pointed at a Todo 5 that does not exist anywhere in the repository; they now describe the prop instead.
The follow-up is complete, so the documents now describe the tree that exists and no longer mix finished work with open work. - ARCHITECTURE.md gains the rule that a feature must not import a component, a lifecycle-and-asset-ownership section (construct/destroy contract, generation-guarded loads, the single-owner proxy argument), the layout and fade relocation, the new setup modules, and a plain/generated CSS section that matches the code. - DESIGN.md stops describing chatStyles.ts as the last remaining exception, and records the styling stack, iconography and surface ownership. - REFACTOR_PLAN.md closes every phase: phase 6 step 2 is closed without code movement with the reason, phase 6 step 3 is resolved by the service-layer move, phase 2's outcome no longer reads as open, and the stale statement about Hugeicons having no official Solid package is struck through with the correction. - REFACTOR_AUDIT.md gains a Post-refactor state section that keeps the original scores, records the after-score and the final owner per issue, and lists the two problems the audit did not predict. - REFACTORING_FOLLOWUP.md gains the completion report required by its section 47, including the items intentionally left unchanged and the measured verification results.
A review pass over the follow-up found two owners that released less than they started. LayoutManager.cleanup() removed the stylesheet but left a smooth follow's animation frame scheduled; it now cancels the frame and drops the follow state. PreviewRuntime.initialize() reset its destroyed flag, so a destroyed runtime could be re-initialized and re-claim the proxy flag and the shared stores; destruction is terminal there too, which is what the application contract already assumed. clearAllCaches() keeps the paint catalogue and the user-to-paint assignments on purpose, so its comment now says what it clears and why, instead of claiming every channel- and user-bound cache. Both new behaviours have tests: cleanup cancels the frame it started and removes the stylesheet, and a destroyed preview runtime cannot be initialized again.
Adds the two teardown fixes, names the four third-party badge services and the retained 7TV paint data among the items intentionally left unchanged, and refreshes the test count.
Collaborator
|
Dinastiya |
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.
Completes
documents/REFACTORING_FOLLOWUP.md. It builds on the first pass inPR #15 — those 28 commits are still in this branch and are described there; this
PR adds 15 commits that close the follow-up's completion criteria.
Panda CSS is not part of this PR, per the follow-up's hard constraint.
Review guide — the 15 follow-up commits
Read top to bottom. The last column is how the change was verified, so the
evidence can be judged against the risk.
3baf16adocs(agents): publish the engineering rules at the repository rootcd17aaerefactor(icons): render through the official hugeicons solid package<svg>on the setup page captured before and after: 65/65 identical geometry, font size, stroke width and colour; only attribute difference is the vendor's explicitcolor="currentColor"5121bf4refactor(architecture): remove the setup layer inversionsc=linaryx, Kick returns 3 suggestionsa7f6fcefix(chat): make application teardown idempotent and terminalbfd8102fix(chat): stop stale asset loads from repopulating reset stores96200bdfix(chat): let the preview runtime release the document state it claims0890919refactor(setup): make the document lock own state instead of a colour/status, re-applied on a setup route1f5c508refactor(chat): move the layout and fade runtimes beside their owner#chat-layoutpublished,layout-vertical/layout-horizontal/layout-reverseapplied, 9 messages, one#chat-animations27b6096fix(chat): release the generated paint stylesheet with its runtimefeb4e11fix(setup): dispose the preview synchronizer on route cleanupdisposewas already covered bytests/previewSync.test.ts; the route simply never called itadabb32test(chat): cover re-applying the overlay styles after cleanup58a017bdocs(debug): drop the stale todo references in the perf panel36b2206fix(chat): release the last two runtime resources at teardown119e083docs(refactor): reconcile the architecture and close the follow-up0ef2d54docs(refactor): record the review findings in the completion reportCommits worth the most scrutiny: 5 (lifecycle race), 4 (destruction
contract), 6 (preview teardown), 2 (the icon renderer swap).
What the follow-up changed
Agent documentation.
AGENTS.mdnow carries the engineering rules directly —layer direction, lifecycle ownership, primitive and icon boundaries, the styling
and custom-property contract, accessibility and OBS constraints, public
contracts, dependency and test policy — and requires repository-wide refactors to
read
REFACTORING.mdandREFACTORING_FOLLOWUP.md.Icons. The custom Hugeicons payload renderer is gone. The wrapper is now a
thin project layer over the vendor's
HugeiconsIcon(@hugeicons/solid-js),owning
currentColor, the1emdefault size and decorative-by-defaultaccessibility. The icon payload keeps its own stroke width, because forcing a
root default would stroke a fill-only icon.
Dependency direction.
SetupSectionIdmoved tofeatures/setup/model/setupSections.ts, so the route, the layout and the searchshare it without importing each other. Two pure helpers left JSX modules that now
import the client-only vendor renderer (the channel search →
services/setup/channelSearch.ts, the label resolver →components/setup/setupText.ts), which also collapsed a duplicated loginnormalization into
services/setup/logins.ts.Lifecycle ownership.
ChatOverlayApplication.destroy()releases what itsconstructor built, is idempotent, and is terminal. Every singleton the runtime
fills carries a generation that
reset()bumps, so a response that resolvesafter teardown cannot commit into the next runtime's store. The preview runtime
releases the proxy flag and the shared asset stores it claimed — and only those.
Layout and fade.
utils/ui/layoutUtils.tsandfadeUtils.tsmoved toservices/chat/runtime/, which is the layer that owns them. The second, no-opLayoutManagerinutils/ui/layoutManager.tsis deleted along with the twocalls that fed it;
utils/ui/no longer exists.Setup document lock. The literal colour is gone: the helper sets
data-setup-documentand the setup stylesheet owns the rules. The inline colourwas provably dead —
app.cssforcesbackground: transparent !importantforOBS, and
.setup-rootpaints the surface.Global state. The 7TV paint stylesheet is removed with the runtime that
generated it; the setup route disposes its preview synchronizer; a smooth-follow
animation frame is cancelled by the manager's cleanup.
Documentation.
ARCHITECTURE.mdandDESIGN.mddescribe the tree thatexists;
REFACTOR_PLAN.mdhas no open or stale status;REFACTOR_AUDIT.mdgains a "Post-refactor state" section that keeps every original score and adds
the after-score;
REFACTORING_FOLLOWUP.mdcarries the completion report.Bugs found by the follow-up
reset but not what was in flight, so a late response repopulated the next
runtime's store.
chatFeatureIntegrationcould even mark itself initializedafter being destroyed and then block the next runtime.
destroy()beforestart()leaked both runtimes the constructor built.not the demo's emotes, badges or paints.
had just released.
would have kept writing into a detached element.
navigation timer could fire after the route was left.
LayoutManager.cleanup()left a smooth-follow animation frame scheduled.tests once the icon module imported the client-only vendor renderer.
Known outstanding
renderMessageContent.tsphysical splitChatConfigtwo-hop barrel through~/utils/chatBreaking changes
None. No query parameter changed, so deployed OBS browser sources keep
working. The stored setup keys are untouched.
tests/setupConfig.test.tsandtests/formValues.test.tspin the config round trip, andbuildOverlayUrlremains the single producer of the overlay link.
Verification
Browser checks (Playwright, kept outside the repository): setup page and its
icons, settings search, channel persistence and overlay URL generation, the
preview iframe and live/demo switching, reduced motion, route cleanup, the
overlay's transparent surfaces and rendering (messages, badges, events,
gigantified emotes, replies, emote modifiers, stroke and shadow presets,
horizontal layout, animations), and the accessibility of the icon migration
(65 icons all decorative, 36 icon-only buttons all named).
Bundle impact of the icon switch:
uichunk +2.8 kB for the vendor renderer,setupchunk −0.5 kB for the removed payload renderer, net ≈ +2.3 kB. Subpathimports are kept, and the built output contains no trace of an unused icon, so
the barrel is not bundled.
Reviewing the first pass
The 28 commits from PR #15 are included and unchanged. Its description has the
per-commit review table for them; the phase-by-phase outcomes are in
documents/REFACTOR_PLAN.md, and the per-issue final state is indocuments/REFACTOR_AUDIT.md.