refactor: Extract pure functions into core modules #4252
Closed
+346
−40
Conversation
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
- Add src/core/calculations.ts with pure calculation utilities - Add src/core/audio-processing.ts with pure audio processing functions - Add src/core/audio-loader.ts with pure audio loading functions - Refactor src/renderer-utils.ts to use core functions, eliminating duplication This is part of the incremental migration from PR #4241, extracting pure function code out of imperative shells for better testability and maintainability. All functions are pure (no side effects, deterministic) and fully typed with comprehensive JSDoc documentation.
katspaugh
changed the title
Deploying wavesurfer-js with
|
| Latest commit: |
2149d20
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://6908f884.wavesurfer-js.pages.dev |
| Branch Preview URL: | https://feat-extract-pure-functions.wavesurfer-js.pages.dev |
- Add 76 unit tests for core/calculations.ts (41 tests) - Add tests for core/audio-processing.ts (21 tests) - Add tests for core/audio-loader.ts (14 tests, 3 skipped) - Achieve 91% code coverage for core modules - 100% coverage for calculations.ts - 99% coverage for audio-processing.ts - 70% coverage for audio-loader.ts (integration tests skipped) Total: 70 passing, 3 skipped, 3 failed (async mocking complexity)
- Remove unused 'channel' parameter in getChannelData mocks - Fixes eslint @typescript-eslint/no-unused-vars errors
- Remove src/core/audio-loader.ts (duplicated Decoder + Fetcher functionality) - Refactor decoder.ts to use needsNormalization and findMaxAmplitude from core/audio-processing - Fix floating point precision in tests (use toBeCloseTo) - Remove audio-loader tests (functionality already covered by decoder tests) - All 372 unit tests passing - Zero duplication between decoder.ts and audio-processing.ts
BREAKING: Remove unused functions from core modules - calculations.ts: Keep only 4 functions (was 17) * getRelativePointerPosition * clampToUnit * calculateScrollPercentages * calculateWaveformLayout - audio-processing.ts: Keep only 2 functions (was 9) * findMaxAmplitude * needsNormalization All kept functions are actively used by: - renderer-utils.ts (uses all 4 calculation functions) - decoder.ts (uses both audio-processing functions) Tests updated to match. All 323 tests passing.
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.
🎯 Overview
This PR extracts pure functions from imperative shells into dedicated core modules, part of the incremental migration from PR #4241.
📦 What's New
New Modules Added
Created 3 new pure function modules in
src/core/:src/core/calculations.ts(357 lines)src/core/audio-processing.ts(318 lines)src/core/audio-loader.ts(216 lines)Refactored Existing Code
src/renderer-utils.tsto use pure functions fromcore/calculations.tsclampToUnit()getRelativePointerPosition()calculateWaveformLayout()calculateScrollPercentages()✅ Benefits
🧪 Testing
🔗 Related
Part of incremental migration from #4241 (reactive architecture refactoring).
📋 Checklist
yarn lint)