Skip to content

fix: remove redundant refetchFn() call and fragile BehaviorSubject handling#73

Merged
michaelbe812 merged 1 commit into
mainfrom
fix/redundant-behaviorsubject-handling
Sep 16, 2025
Merged

fix: remove redundant refetchFn() call and fragile BehaviorSubject handling#73
michaelbe812 merged 1 commit into
mainfrom
fix/redundant-behaviorsubject-handling

Conversation

@michaelbe812

Copy link
Copy Markdown
Owner

Summary

  • Eliminate redundant double-call to strategy.refetchFn() in mergeRefetchStrategies function
  • Remove fragile instanceof BehaviorSubject check that doesn't work reliably with piped observables
  • Simplify implementation by letting individual strategies handle their own skip(1) logic

Context

This PR addresses issue #48 which identified performance and logic issues in the mergeRefetchStrategies function.

Problems Fixed

  1. Redundant Function Calls: The function was calling strategy.refetchFn() twice - once for the pipe operation and once for the BehaviorSubject check. This was wasteful and could cause side effects with cold streams.

  2. Fragile BehaviorSubject Detection: The instanceof BehaviorSubject check was unreliable and created unnecessary coupling between the merge logic and individual strategies.

Changes Made

  • Modified mergeRefetchStrategies to call refetchFn() only once per strategy
  • Removed the BehaviorSubject instanceof check and associated skip logic
  • Simplified the function to filter null/undefined strategies first, then map to observables
  • Added comprehensive tests to verify single-call behavior and proper BehaviorSubject handling

Testing

  • All existing tests pass ✅
  • Added new test to verify refetchFn() is called exactly once per strategy
  • Added test to confirm BehaviorSubject initial values are still skipped correctly (handled by the strategy itself)
  • Full test suite passes with no regressions

Benefits

  • Better performance (no redundant function calls)
  • More reliable logic (no fragile instanceof checks)
  • Cleaner separation of concerns between merge logic and individual strategies
  • Simpler, more maintainable code

Fixes #48

- Eliminate double-call to strategy.refetchFn() in mergeRefetchStrategies
- Remove fragile instanceof BehaviorSubject check
- Let individual strategies handle their own skip(1) logic
- Add comprehensive tests for single-call behavior and BehaviorSubject handling

Fixes #48
@michaelbe812
michaelbe812 merged commit d4e5dd4 into main Sep 16, 2025
1 check passed
@michaelbe812
michaelbe812 deleted the fix/redundant-behaviorsubject-handling branch September 16, 2025 12:27
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.

Fix redundant and fragile BehaviorSubject handling in merge-refetch-strategies

1 participant