refactor: shrink YAGNI signatures - #194
Merged
Merged
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Four mechanical shrinks from the v1.8.0 ponytail audit. Pure refactor, no behavior change. 1. Drop the single-impl cursor interface. CursorNativeAdapter had exactly one implementation (TelemetryCursorAdapter); the interface is gone and the class carries the shape directly. electron/native-bridge/cursor/adapter.ts is deleted; CursorTelemetryLoadResult now lives next to the class. cursorService.ts and ipc/nativeBridge.ts import the class (or the new type) instead. 2. Remove unused parameters from clamp helpers. clampFocusToDepth(focus, _depth) becomes clampFocus(focus) (renamed; the depth argument was never read). clampFocusToStage(focus, depth, _stageSize) becomes clampFocusToStage(focus, depth) (the stage size was never read; depth is still required for the bounds). 3. Collapse parseCustomPlaybackSpeedInput. The 22-line re-implementation of Number() + clamp (with a draft round-trip that stripped invalid characters and rebuilt the input) is replaced with a one-liner and a single finite-check. The discriminated union keeps empty/too-fast/too-slow/valid; the only valid path now carries the speed. Two tests that pinned the old round-trip are updated to match the new shape. 4. NativeBridgeStateStore class -> plain object factory. The 4-method class is replaced with a NativeBridgeState interface and a createNativeBridgeState(platform) factory. The returned object is a plain shape with the four setters as direct property assignments plus a getState() accessor. The data shape becomes an internal NativeBridgeStateData (not exported) since only the factory and the type need it.
The clamp formula was re-inlined byte-identical to clampPlaybackSpeed, which is
still live in types.ts — two copies of the same rounding rule, and this one
feeds the native scene.
rawValue was also widened to string | null | undefined in a PR shrinking YAGNI
signatures; the sole caller passes a useState("") value that is never nullish.
EtienneLescot
force-pushed
the
ponytail/shrink-yagni-signatures
branch
from
July 29, 2026 08:33
c1e653c to
9af8695
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.
refactor: shrink YAGNI signatures
Four mechanical shrinks from the v1.8.0 ponytail audit. Pure refactor, no
behavior change.
Changes
Drop the single-impl cursor interface.
CursorNativeAdapterhad exactly one implementation (TelemetryCursorAdapter); the interface is gone and the class carries the shape directly.electron/native-bridge/cursor/adapter.tsis deleted;CursorTelemetryLoadResultnow lives next to the class.cursorService.tsandipc/nativeBridge.tsimport the class (or the new type) instead.Remove unused parameters from clamp helpers.
clampFocusToDepth(focus, _depth)→clampFocus(focus)(renamed; the depth argument was never read).clampFocusToStage(focus, depth, _stageSize)→clampFocusToStage(focus, depth)(the stage size was never read;depthis still required for the bounds).Collapse
parseCustomPlaybackSpeedInput. The 22-line re-implementation ofNumber() + clamp(with a draft round-trip that stripped invalid characters and rebuilt the input) is replaced with the spec's one-liner and a single finite-check. The discriminated union keepsempty/too-fast/too-slow/valid; the onlyvalidpath now carries the speed. Two tests that pinned the old round-trip (the "1.2.3 → 1.23" parse and the "1,1 → draft: 1.1" normalization) are updated to match the new shape — the behavior change is exactly the spec's "drop the round-trip artifact" intent.NativeBridgeStateStoreclass → plain object factory. The 4-method class is replaced with aNativeBridgeStateinterface and acreateNativeBridgeState(platform)factory. The returned object is a plain shape with the four setters as direct property assignments plus agetState()accessor. The data shape becomes an internalNativeBridgeStateData(not exported) since only the factory and the type need it.Type of change
Release impact
Desktop impact
Testing