fix(sync): rename skipDuringSync to skipWhileApplyingRemoteOps

- Rename operator to be more descriptive of actual behavior
- Add deprecated alias for backwards compatibility
- Remove inner guards from effects - inject() only works during class
  initialization, not inside switchMap callbacks at runtime
- Keep outer guards only (at createEffect level)
- Update ESLint rule to accept both names
- Add comprehensive unit tests for the operator (6 tests)

Key insight: The skipWhileApplyingRemoteOps() operator uses inject()
internally, which only works within Angular's injection context (during
class field initialization when createEffect() is called). Using it
inside switchMap callbacks causes NG0203 errors at runtime.

Also: currentTaskId$ is local UI state that doesn't sync between devices.
The operation log syncs entities (tasks, projects, tags), not UI state.
Therefore distinctUntilChanged() is sufficient for filtering spurious
emissions on inner observables.
This commit is contained in:
Johannes Millan 2025-12-28 18:14:34 +01:00
parent 140c46d40f
commit b3022c7285
11 changed files with 255 additions and 51 deletions

View file

@ -14,7 +14,8 @@
* ## Solution
*
* Effects that USE SELECTORS AS THE PRIMARY SOURCE should include one of:
* - `skipDuringSync()` operator
* - `skipWhileApplyingRemoteOps()` operator (preferred)
* - `skipDuringSync()` operator (deprecated alias)
* - `filter(() => !this.hydrationState.isApplyingRemoteOps())`
*
* ## What This Rule Does NOT Flag
@ -57,7 +58,7 @@ module.exports = {
},
messages: {
missingHydrationGuard:
'Selector-based effect is missing hydration guard. Add skipDuringSync() or filter with isApplyingRemoteOps() to prevent duplicate operations during sync replay.',
'Selector-based effect is missing hydration guard. Add skipWhileApplyingRemoteOps() or filter with isApplyingRemoteOps() to prevent duplicate operations during sync replay.',
},
schema: [],
},
@ -267,7 +268,9 @@ module.exports = {
// Check for hydration guards
return (
chainText.includes('skipDuringSync') || chainText.includes('isApplyingRemoteOps')
chainText.includes('skipWhileApplyingRemoteOps') ||
chainText.includes('skipDuringSync') ||
chainText.includes('isApplyingRemoteOps')
);
};

View file

@ -26,7 +26,19 @@ ruleTester.run('require-hydration-guard', rule, {
`,
},
// Selector with skipDuringSync guard - should NOT flag
// Selector with skipWhileApplyingRemoteOps guard (preferred) - should NOT flag
{
code: `
createEffect(() =>
this._store$.select(mySelector).pipe(
skipWhileApplyingRemoteOps(),
tap(() => this.doSomething())
)
)
`,
},
// Selector with skipDuringSync guard (deprecated alias) - should NOT flag
{
code: `
createEffect(() =>

View file

@ -3,7 +3,7 @@ import { createEffect } from '@ngrx/effects';
import { LOCAL_ACTIONS } from '../../../util/local-actions.token';
import { EMPTY, Observable, timer } from 'rxjs';
import { distinctUntilChanged, switchMap, tap, withLatestFrom } from 'rxjs/operators';
import { skipDuringSync } from '../../../util/skip-during-sync.operator';
import { skipWhileApplyingRemoteOps } from '../../../util/skip-during-sync.operator';
import { Store } from '@ngrx/store';
import { selectIsDominaModeConfig } from '../../config/store/global-config.reducer';
import { selectCurrentTask } from '../../tasks/store/task.selectors';
@ -17,7 +17,8 @@ export class DominaModeEffects {
dominaMode$: Observable<unknown> = createEffect(
() =>
this._store$.select(selectIsDominaModeConfig).pipe(
skipDuringSync(),
// Outer guard: skip config changes during sync
skipWhileApplyingRemoteOps(),
distinctUntilChanged(),
switchMap((cfg) =>
cfg.isEnabled && cfg.voice

View file

@ -3,7 +3,7 @@ import { createEffect, ofType } from '@ngrx/effects';
import { LOCAL_ACTIONS } from '../../../util/local-actions.token';
import { Store } from '@ngrx/store';
import { combineLatest, EMPTY, of } from 'rxjs';
import { skipDuringSync } from '../../../util/skip-during-sync.operator';
import { skipWhileApplyingRemoteOps } from '../../../util/skip-during-sync.operator';
import {
distinctUntilChanged,
filter,
@ -61,12 +61,12 @@ export class FocusModeEffects {
this.store.select(selectFocusModeConfig),
this.store.select(selectIsFocusModeEnabled),
]).pipe(
// Prevent auto-showing overlay during sync - config/task state changes from
// remote ops would otherwise trigger the overlay unexpectedly
skipDuringSync(),
// Outer guard: skip config changes during sync
skipWhileApplyingRemoteOps(),
switchMap(([cfg, isFocusModeEnabled]) =>
isFocusModeEnabled && cfg?.isSyncSessionWithTracking && !cfg?.isStartInBackground
? this.taskService.currentTaskId$.pipe(
// currentTaskId$ is local UI state (not synced), so distinctUntilChanged is sufficient
distinctUntilChanged(),
filter((id) => !!id),
map(() => actions.showFocusOverlay()),
@ -83,12 +83,12 @@ export class FocusModeEffects {
this.store.select(selectFocusModeConfig),
this.store.select(selectIsFocusModeEnabled),
]).pipe(
// Prevent auto-starting/unpausing focus session during sync - currentTaskId
// changes from remote ops would otherwise start sessions unexpectedly
skipDuringSync(),
// Outer guard: skip config changes during sync
skipWhileApplyingRemoteOps(),
switchMap(([cfg, isFocusModeEnabled]) =>
isFocusModeEnabled && cfg?.isSyncSessionWithTracking
? this.taskService.currentTaskId$.pipe(
// currentTaskId$ is local UI state (not synced), so distinctUntilChanged is sufficient
distinctUntilChanged(),
filter((taskId) => !!taskId),
withLatestFrom(
@ -123,7 +123,7 @@ export class FocusModeEffects {
// CRITICAL: Prevent cascading dispatches during sync that cause app freeze.
// Without this, rapid currentTaskId changes from remote ops trigger pairwise()
// which dispatches pauseFocusSession repeatedly, overwhelming the store.
skipDuringSync(),
skipWhileApplyingRemoteOps(),
pairwise(),
withLatestFrom(
this.store.select(selectFocusModeConfig),
@ -219,7 +219,7 @@ export class FocusModeEffects {
// Only triggers when timer STOPS (isRunning becomes false) with elapsed >= duration
detectSessionCompletion$ = createEffect(() =>
this.store.select(selectors.selectTimer).pipe(
skipDuringSync(),
skipWhileApplyingRemoteOps(),
withLatestFrom(this.store.select(selectors.selectMode)),
// Only consider emissions where timer just stopped running
distinctUntilChanged(
@ -241,7 +241,7 @@ export class FocusModeEffects {
detectBreakTimeUp$ = createEffect(
() =>
this.store.select(selectors.selectTimer).pipe(
skipDuringSync(),
skipWhileApplyingRemoteOps(),
filter(
(timer) =>
timer.purpose === 'break' &&
@ -518,7 +518,7 @@ export class FocusModeEffects {
createEffect(
() =>
this.store.select(selectors.selectProgress).pipe(
skipDuringSync(),
skipWhileApplyingRemoteOps(),
withLatestFrom(this.store.select(selectors.selectIsRunning)),
tap(([progress, isRunning]) => {
window.ea.setProgressBar({
@ -559,7 +559,7 @@ export class FocusModeEffects {
this.store.select(selectFocusModeConfig),
this.store.select(selectIsFocusModeEnabled),
]).pipe(
skipDuringSync(),
skipWhileApplyingRemoteOps(),
tap(
([
isSessionRunning,
@ -798,7 +798,7 @@ export class FocusModeEffects {
playTickSound$ = createEffect(
() =>
this.store.select(selectors.selectTimer).pipe(
skipDuringSync(),
skipWhileApplyingRemoteOps(),
filter(
(timer) => timer.isRunning && timer.purpose === 'work' && timer.elapsed > 0,
),

View file

@ -1,7 +1,7 @@
import { inject, Injectable } from '@angular/core';
import { createEffect, ofType } from '@ngrx/effects';
import { LOCAL_ACTIONS } from '../../../util/local-actions.token';
import { skipDuringSync } from '../../../util/skip-during-sync.operator';
import { skipWhileApplyingRemoteOps } from '../../../util/skip-during-sync.operator';
import { ChromeExtensionInterfaceService } from '../../../core/chrome-extension-interface/chrome-extension-interface.service';
import { WorkContextService } from '../../work-context/work-context.service';
import { TaskService } from '../../tasks/task.service';
@ -107,7 +107,8 @@ export class IdleEffects {
// while the dialog is open, preventing the flickering between two different values.
triggerIdleWhenEnabled$ = createEffect(() =>
this._store.select(selectIdleConfig).pipe(
skipDuringSync(),
// Outer guard: skip config changes during sync
skipWhileApplyingRemoteOps(),
switchMap(
({
isEnableIdleTimeTracking,
@ -145,7 +146,9 @@ export class IdleEffects {
handleIdleInit$ = createEffect(() =>
this._store.select(selectIsIdle).pipe(
skipDuringSync(),
// Guard: skip idle state changes during sync - CRITICAL because this effect
// has data mutations (removeTimeSpent, setCurrentId, decreaseCounterToday)
skipWhileApplyingRemoteOps(),
distinctUntilChanged(),
switchMap((isIdle) => iif(() => isIdle, of(isIdle), EMPTY)),
withLatestFrom(

View file

@ -14,7 +14,7 @@ import {
switchMap,
tap,
} from 'rxjs/operators';
import { skipDuringSync } from '../../../util/skip-during-sync.operator';
import { skipWhileApplyingRemoteOps } from '../../../util/skip-during-sync.operator';
import { BannerId } from '../../../core/banner/banner.model';
import { T } from '../../../t.const';
import { LocaleDatePipe } from 'src/app/ui/pipes/locale-date.pipe';
@ -49,7 +49,8 @@ export class ReminderCountdownEffects {
() =>
this._dataInitStateService.isAllDataLoadedInitially$.pipe(
concatMap(() => this._store.select(selectReminderConfig)),
skipDuringSync(),
// Outer guard: skip config changes during sync
skipWhileApplyingRemoteOps(),
switchMap((reminderCfg) =>
reminderCfg.isCountdownBannerEnabled
? combineLatest([
@ -67,15 +68,14 @@ export class ReminderCountdownEffects {
);
}),
switchMap((dueTasks) =>
this._store
.select(selectCurrentTaskId)
.pipe(distinctUntilChanged())
.pipe(
map((currentId) => ({
currentId,
dueTasks: dueTasks.filter((t) => t.id !== currentId),
})),
),
this._store.select(selectCurrentTaskId).pipe(
// currentTaskId is local UI state (not synced), so distinctUntilChanged is sufficient
distinctUntilChanged(),
map((currentId) => ({
currentId,
dueTasks: dueTasks.filter((t) => t.id !== currentId),
})),
),
),
tap(({ dueTasks }) => this._showBanner(dueTasks)),
)

View file

@ -30,7 +30,7 @@ import { GlobalConfigService } from '../../config/global-config.service';
import { playDoneSound } from '../util/play-done-sound';
import { Task } from '../task.model';
import { EMPTY } from 'rxjs';
import { skipDuringSync } from '../../../util/skip-during-sync.operator';
import { skipWhileApplyingRemoteOps } from '../../../util/skip-during-sync.operator';
import { selectProjectById } from '../../project/store/project.selectors';
import { Router } from '@angular/router';
import { NavigateToTaskService } from '../../../core-ui/navigate-to-task/navigate-to-task.service';
@ -122,13 +122,13 @@ export class TaskUiEffects {
timeEstimateExceeded$ = createEffect(
() =>
this._store$.pipe(select(selectConfigFeatureState)).pipe(
// Prevent spurious notifications during sync - currentTask state changes
// from remote ops would otherwise trigger "time estimate exceeded" alerts
skipDuringSync(),
// Outer guard: skip config changes during sync
skipWhileApplyingRemoteOps(),
switchMap((globalCfg) =>
globalCfg && globalCfg.timeTracking.isNotifyWhenTimeEstimateExceeded
? // reset whenever the current taskId changes (but no the task data, which is polled afterwards)
this._store$.pipe(select(selectCurrentTaskId)).pipe(
// currentTaskId is local UI state (not synced), so distinctUntilChanged is sufficient
distinctUntilChanged(),
switchMap(() =>
this._store$.pipe(
@ -156,9 +156,8 @@ export class TaskUiEffects {
timeEstimateExceededDismissBanner$ = createEffect(
() =>
this._store$.pipe(select(selectConfigFeatureState)).pipe(
// Prevent unexpected banner dismissal during sync - currentTaskId changes
// from remote ops would otherwise dismiss the time estimate exceeded banner
skipDuringSync(),
// Outer guard: skip config changes during sync
skipWhileApplyingRemoteOps(),
switchMap((globalCfg) =>
globalCfg && globalCfg.timeTracking.isNotifyWhenTimeEstimateExceeded
? this._bannerService.activeBanner$.pipe(
@ -166,6 +165,7 @@ export class TaskUiEffects {
activeBanner?.id === BannerId.TimeEstimateExceeded
? this._store$.pipe(
select(selectCurrentTaskId),
// currentTaskId is local UI state (not synced), so distinctUntilChanged is sufficient
distinctUntilChanged(),
skip(1),
)

View file

@ -2,7 +2,7 @@ import { Injectable, inject } from '@angular/core';
import { createEffect, ofType } from '@ngrx/effects';
import { LOCAL_ACTIONS } from '../../../util/local-actions.token';
import { filter, map, tap, withLatestFrom, startWith } from 'rxjs/operators';
import { skipDuringSync } from '../../../util/skip-during-sync.operator';
import { skipWhileApplyingRemoteOps } from '../../../util/skip-during-sync.operator';
import { setSelectedTask } from '../../tasks/store/task.actions';
import { TaskService } from '../../tasks/task.service';
import { BannerId } from '../../../core/banner/banner.model';
@ -49,7 +49,7 @@ export class WorkContextEffects {
startWith(this._router.url), // Handle initial page load
// Prevent dispatching setActiveWorkContext during sync - startWith() emits
// immediately on subscription which could interfere with sync state hydration
skipDuringSync(),
skipWhileApplyingRemoteOps(),
filter(
(url) =>
!!url.match(/(schedule)$/) ||

View file

@ -9,7 +9,7 @@ import { HydrationStateService } from '../op-log/apply/hydration-state.service';
*
* ## Why This Exists
*
* `skipDuringSync()` only blocks during `isApplyingRemoteOps()` - the window
* `skipWhileApplyingRemoteOps()` only blocks during `isApplyingRemoteOps()` - the window
* when operations are being applied to the store. However, there's a timing gap:
*
* 1. Tab gains focus sync triggers
@ -23,7 +23,7 @@ import { HydrationStateService } from '../op-log/apply/hydration-state.service';
*
* ## When to Use
*
* Use this operator instead of `skipDuringSync()` for selector-based effects that:
* Use this operator instead of `skipWhileApplyingRemoteOps()` for selector-based effects that:
* - Modify frequently-synced entities (TODAY_TAG, etc.)
* - Perform "repair" or "consistency" operations
* - Could create operations that immediately conflict with remote changes
@ -44,11 +44,11 @@ import { HydrationStateService } from '../op-log/apply/hydration-state.service';
* }
* ```
*
* ## Comparison with skipDuringSync()
* ## Comparison with skipWhileApplyingRemoteOps()
*
* | Operator | Suppresses during | Use case |
* |----------|-------------------|----------|
* | skipDuringSync() | isApplyingRemoteOps only | General selector-based effects |
* | skipWhileApplyingRemoteOps() | isApplyingRemoteOps only | General selector-based effects |
* | skipDuringSyncWindow() | isApplyingRemoteOps + cooldown | Effects that modify shared entities |
*/
export const skipDuringSyncWindow = <T>(): MonoTypeOperatorFunction<T> => {

View file

@ -0,0 +1,180 @@
import { TestBed } from '@angular/core/testing';
import { Subject } from 'rxjs';
import { skipWhileApplyingRemoteOps, skipDuringSync } from './skip-during-sync.operator';
import { HydrationStateService } from '../op-log/apply/hydration-state.service';
describe('skipWhileApplyingRemoteOps', () => {
let hydrationStateService: jasmine.SpyObj<HydrationStateService>;
let source$: Subject<number>;
beforeEach(() => {
hydrationStateService = jasmine.createSpyObj('HydrationStateService', [
'isApplyingRemoteOps',
]);
hydrationStateService.isApplyingRemoteOps.and.returnValue(false);
TestBed.configureTestingModule({
providers: [{ provide: HydrationStateService, useValue: hydrationStateService }],
});
source$ = new Subject<number>();
});
it('should pass emissions when not applying remote ops', (done) => {
hydrationStateService.isApplyingRemoteOps.and.returnValue(false);
TestBed.runInInjectionContext(() => {
const results: number[] = [];
source$.pipe(skipWhileApplyingRemoteOps()).subscribe({
next: (val) => results.push(val),
complete: () => {
expect(results).toEqual([1, 2, 3]);
done();
},
});
});
source$.next(1);
source$.next(2);
source$.next(3);
source$.complete();
});
it('should skip emissions when applying remote ops', (done) => {
hydrationStateService.isApplyingRemoteOps.and.returnValue(true);
TestBed.runInInjectionContext(() => {
const results: number[] = [];
source$.pipe(skipWhileApplyingRemoteOps()).subscribe({
next: (val) => results.push(val),
complete: () => {
expect(results).toEqual([]);
done();
},
});
});
source$.next(1);
source$.next(2);
source$.next(3);
source$.complete();
});
it('should dynamically filter based on current state', (done) => {
TestBed.runInInjectionContext(() => {
const results: number[] = [];
source$.pipe(skipWhileApplyingRemoteOps()).subscribe({
next: (val) => results.push(val),
complete: () => {
expect(results).toEqual([1, 3, 5]);
done();
},
});
});
// Not syncing - should pass
hydrationStateService.isApplyingRemoteOps.and.returnValue(false);
source$.next(1);
// Start syncing - should skip
hydrationStateService.isApplyingRemoteOps.and.returnValue(true);
source$.next(2);
// Still syncing - should skip
source$.next(2.5);
// Stop syncing - should pass
hydrationStateService.isApplyingRemoteOps.and.returnValue(false);
source$.next(3);
// Start syncing again - should skip
hydrationStateService.isApplyingRemoteOps.and.returnValue(true);
source$.next(4);
// Stop syncing - should pass
hydrationStateService.isApplyingRemoteOps.and.returnValue(false);
source$.next(5);
source$.complete();
});
it('should work with different value types', (done) => {
hydrationStateService.isApplyingRemoteOps.and.returnValue(false);
TestBed.runInInjectionContext(() => {
const stringSource$ = new Subject<string>();
const results: string[] = [];
stringSource$.pipe(skipWhileApplyingRemoteOps()).subscribe({
next: (val) => results.push(val),
complete: () => {
expect(results).toEqual(['a', 'b', 'c']);
done();
},
});
stringSource$.next('a');
stringSource$.next('b');
stringSource$.next('c');
stringSource$.complete();
});
});
});
describe('skipDuringSync (deprecated alias)', () => {
let hydrationStateService: jasmine.SpyObj<HydrationStateService>;
let source$: Subject<number>;
beforeEach(() => {
hydrationStateService = jasmine.createSpyObj('HydrationStateService', [
'isApplyingRemoteOps',
]);
hydrationStateService.isApplyingRemoteOps.and.returnValue(false);
TestBed.configureTestingModule({
providers: [{ provide: HydrationStateService, useValue: hydrationStateService }],
});
source$ = new Subject<number>();
});
it('should work identically to skipWhileApplyingRemoteOps', (done) => {
hydrationStateService.isApplyingRemoteOps.and.returnValue(false);
TestBed.runInInjectionContext(() => {
const results: number[] = [];
source$.pipe(skipDuringSync()).subscribe({
next: (val) => results.push(val),
complete: () => {
expect(results).toEqual([1, 2, 3]);
done();
},
});
});
source$.next(1);
source$.next(2);
source$.next(3);
source$.complete();
});
it('should skip emissions when applying remote ops', (done) => {
hydrationStateService.isApplyingRemoteOps.and.returnValue(true);
TestBed.runInInjectionContext(() => {
const results: number[] = [];
source$.pipe(skipDuringSync()).subscribe({
next: (val) => results.push(val),
complete: () => {
expect(results).toEqual([]);
done();
},
});
});
source$.next(1);
source$.next(2);
source$.next(3);
source$.complete();
});
});

View file

@ -4,8 +4,8 @@ import { filter } from 'rxjs/operators';
import { HydrationStateService } from '../op-log/apply/hydration-state.service';
/**
* RxJS operator that skips emissions during remote operation application
* (hydration/sync replay).
* RxJS operator that skips emissions while remote operations are being applied
* to the store (during hydration/sync replay).
*
* ## Why This Exists
*
@ -22,13 +22,13 @@ import { HydrationStateService } from '../op-log/apply/hydration-state.service';
* ## Usage
*
* ```typescript
* import { skipDuringSync } from '../../../util/skip-during-sync.operator';
* import { skipWhileApplyingRemoteOps } from '../../../util/skip-during-sync.operator';
*
* @Injectable()
* export class MyEffects {
* myEffect$ = createEffect(() =>
* this.store.select(mySelector).pipe(
* skipDuringSync(), // <-- Add this to guard selector-based effects
* skipWhileApplyingRemoteOps(), // <-- Add this to guard selector-based effects
* tap(...),
* )
* );
@ -48,7 +48,12 @@ import { HydrationStateService } from '../op-log/apply/hydration-state.service';
* `createEffect()` is called during class field initialization, which is still
* within an injection context.
*/
export const skipDuringSync = <T>(): MonoTypeOperatorFunction<T> => {
export const skipWhileApplyingRemoteOps = <T>(): MonoTypeOperatorFunction<T> => {
const hydrationState = inject(HydrationStateService);
return filter(() => !hydrationState.isApplyingRemoteOps());
};
/**
* @deprecated Use `skipWhileApplyingRemoteOps` instead. This alias exists for backwards compatibility.
*/
export const skipDuringSync = skipWhileApplyingRemoteOps;