chore(preview): delete six declarations nothing reads - #427
Merged
Conversation
PathGao
force-pushed
the
chore/drop-dead-load-revision-map
branch
from
August 3, 2026 07:45
076205d to
072a366
Compare
`MarkdownViewer.svelte` declares six top-level bindings that the file never mentions again. They read as live state — a container element, a render debounce and its timer handle, a per-tab load revision map, a caret element and its measured offset — so anyone tracing how rendering or scroll sync works has to prove each one is inert before ruling it out. Five were never used at all. `containerEl`, `renderTimeout` and `renderDebounceMs` arrive in b46a283 and `caretEl`/`caretAbsoluteTop` in ef219cf, each already at exactly one occurrence in the commit that adds them: the declaration, and nothing else. `renderDebounceMs = 50` in particular suggests a debounce that does not exist. One is refactor residue. `loadRevisionByTab` had four occurrences when 41de13b (#247) added it here. bb8dbf3 (#274) moved document loading into `documentSession.svelte.ts`, which carries its own map and the three call sites; the declaration here stayed behind. Two identically named maps in two files, one of them dead, is worse than either alone — `scripts/largeFileLoadRevision.test.ts` pins the live one in `documentSession.svelte.ts` and is unaffected. Nothing else changes. `npm run check` and `npm run build` were the check that matters: an unreferenced binding is only safe to remove if the compiler agrees nothing reaches it, including through Svelte's `bind:this`, which is why this is verified by a build rather than by grep alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
PathGao
force-pushed
the
chore/drop-dead-load-revision-map
branch
from
August 3, 2026 07:46
072a366 to
3ce2ae9
Compare
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.
MarkdownViewer.sveltedeclares six top-level bindings the file never mentions again. They read as live state, so anyone tracing how rendering or scroll sync works has to prove each one is inert before ruling it out.let containerEl: HTMLElementb46a283const renderDebounceMs = 50b46a283let renderTimeout: ReturnType<typeof setTimeout> | nullb46a283let caretEl: HTMLElementef219cflet caretAbsoluteTop = 0ef219cfconst loadRevisionByTab = new Map<string, number>()41de13b(#247)Five were never used at all. One occurrence in the commit that introduces them means the declaration and nothing else.
renderDebounceMs = 50next torenderTimeoutis the one most likely to mislead — together they describe a render debounce that does not exist anywhere in the file.One is refactor residue.
loadRevisionByTabhad four occurrences when #247 added it here.bb8dbf3(#274) extracted document loading intosrc/lib/sessions/documentSession.svelte.ts, which carries its own map and all three call sites; the declaration here stayed behind, dropping to one occurrence. Two identically named maps in two files, one of them dead, is worse than either alone — a reader who greps the name finds both.How "unreferenced" was established
Grep alone is not sufficient in a Svelte component:
bind:this={containerEl}in the template is a real use, and a name can be reached from the markup without appearing in the script block. So:src/andscripts/for each name outside this file — none.npm run check(438 files, 0 errors) andnpm run build— the compiler resolving the template is the check that matters. A template reference to a deleted binding fails the build.scripts/largeFileLoadRevision.test.tspins the live map indocumentSession.svelte.tsand is untouched.Verification
Not covered
let/constin this one file. Unused imports, unused function parameters, and dead code in other files were not swept — a wider sweep would be a different change with a different review surface.caretEl/caretAbsoluteTopcame in with split view and scroll syncing (ef219cf). They were dead on arrival, so removing them cannot change scroll sync, but it also does not tell you whether the caret-tracking they were meant for is missing on purpose. That question is left where it was.🤖 Generated with Claude Code