Repository navigation
fix(hr): stop adding a resize listener per horizontal rule - #57
Merged
Merged
Conversation
`useTerminalDimensions` subscribes to the renderer's `resize` event once per call site, so every horizontal rule in a document added a listener. Past 10, Node prints a MaxListenersExceededWarning to stderr — over the live TUI, which shifts the screen mid-scroll. A document with 15 rules held 22 listeners. `Hr` now reads `contentWidth` from `AppStateContext`, as `Table`, `CodeBlock` and `Frontmatter` already do, and sizes the rule to the content column instead of the terminal width it relied on the box to clip.
denolfe
changed the title
Startup benchmark (
|
| build | mean | ratio | verdict |
|---|---|---|---|
| baseline (main@b7ab07d) | 363.7ms ± 8.1ms | — | |
| PR | 360.4ms ± 10.6ms | 0.99× | ✅ ok |
Thresholds: warn ≥ 1.1×, fail ≥ 1.25×. Baseline built from main.
First-frame (large.md, runs=10)
| metric | baseline (main@b7ab07d) | PR | ratio | verdict |
|---|---|---|---|---|
| first-frame | 272.2ms | 266.1ms | 0.98× | ✅ ok |
Thresholds: warn ≥ 1.1×, fail ≥ 1.25×. Baseline built from main.
Scroll (large.md)
| metric | baseline (main@b7ab07d) | PR | ratio | verdict |
|---|---|---|---|---|
| full_mount | 10277.0ms | 10292.0ms | 1.00× | ✅ ok |
| step_p50 | 22.0ms | 22.4ms | 1.02× | ✅ ok |
| step_search_p50 | 22.7ms | 23.2ms | 1.02× | ✅ ok |
| stepMatch_p50 | 16.4ms | 17.4ms | 1.06× | ✅ ok |
| commit | 139.7ms | 147.4ms | 1.06× | ✅ ok |
Thresholds: warn ≥ 1.1×, fail ≥ 1.25×. Baseline built from main.
denolfe
added a commit
that referenced
this pull request
Sep 9, 2026
- feat: copy selected text (#58) (a9f836e) - fix(hr): stop adding a resize listener per horizontal rule (#57) (beedc6d) - ci: add first-frame and scroll benchmarks to the PR bench job (#56) (b7ab07d) - perf: improve first frame and progressive-mount speed, optimize large documents (#54) (3b6d9f2) - chore(ci): pin bun 1.4.1 via mise (#53) (f596175)
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.
What was wrong:
Hrgot its width fromuseTerminalDimensions(), which adds aresizelistener to the renderer every time it is called — so one listener per---in the document.Why it matters: Once the count passes 10, Node prints a
MaxListenersExceededWarningto stderr, on top of the running TUI. That is the visible column shift while scrolling. A 15-rule document held 22 listeners.The fix:
Hrnow readscontentWidthfromuseAppState(), likeTable,CodeBlockandFrontmatteralready do. No listener. Same document now holds 7.The test: mounts a 2-rule and a 40-rule document and asserts the listener count is the same and under 10.