Skip to content

Refactor (packages/ui/src/v2/components/text-shimmer-v2.tsx): Dedupe text shimmer logic - #111

Open
GioChkhaidze wants to merge 3 commits into
CMU-17313Q:mainfrom
GioChkhaidze:shimmer-dedup
Open

GioChkhaidze wants to merge 3 commits into
CMU-17313Q:mainfrom
GioChkhaidze:shimmer-dedup

Conversation

@GioChkhaidze

@GioChkhaidze GioChkhaidze commented Sep 6, 2026

Copy link
Copy Markdown

P1B: Starter Task: Dedupe text shimmer logic

1. Issue

Link: Closes #102

Full Path: packages/ui/src/v2/components/text-shimmer-v2.tsx, packages/ui/src/components/text-shimmer.tsx and newly added packages/ui/src/components/text-shimmer-core.tsx and its test.

What file does: it's a responsible for SolidJS component rendering for text rendering.

Scope: replaced TextShimmerV2 & TextShimmer components with wrapper which has shared core logic from both of them.

Qlty-report: Found 59 lines of similar code in 2 locations (mass = 290) at text-shimmer-v2.tsx:5 in components/text-shimmer.tsx

2. Refactoring

Maintainability: Each update must have been applied to both shimmers and might lead to drift, so with this fix I centralized logic so that it's easier to maintain (boths shimmers are live in session-turn and in basic-tool-v2).

Change: Extracted shared logic from both shimmers to single core shared code and reused it in both (centralized the logic). TextShimmerCore now owns logic and TextShimmer and TextShimmerV2 both reuse it.

Alternative: TextShimmerCore acts as a source of truth and is better for maintainability in long term. I considered removing v1 and using v2 to wrap v1 which would be smaller code change but rather making the codebase dirty in terms of understanding why one version reuses second one. Another reason why my approach is better is that it is cleaner foundation for newer future versions of shimmer reusing the same logic and acting as a source of truth.

3. Validation

Validation: I ran, bun run typecheck, qlty smells --no-snippets (on both wrappers reporting no deduplication), and bun test --coverage --coverage-dir=./coverage showing 33 tests pass (27 + 6 new test) in opencode/packages/ui. The new test-shimmer-core.test.ts imported helpers and configs and checked the mappings, v1 & v2 selectors, executing shared path for both wrappers.

The test coverage showing the lines were executed by the tests.

Tests Added

Lints, typechecks and tests were executed successfully.

Test Lints, typechecks

CI checks pass

All checks pass

My additional tests (27 + 6 new tests all pass in github actions CI in packages/ui)

tests pass in CI

qlty smells --no-snippets packages/ui/src/v2/components/text-shimmer-v2.tsx showing issues BEFORE changes.

Evidence before PR

qlty smells --no-snippets packages/ui/src/v2/components/text-shimmer-v2.tsx showing no reported issues AFTER changes.

Evidence after PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

P1B: Refactor (packages/ui/src/v2/components/text-shimmer-v2.tsx:5): Found 59 lines of similar code in 2 locations (mass = 290)

1 participant