Add hydration guard for selector-based NgRx effects (#6426)

* fix(sync): guard unprotected selector-based effects against sync replay

Add skipWhileApplyingRemoteOps() guards to selector-based effects that
were missing hydration protection, preventing unwanted side effects
during sync/hydration replay:

- tag.effects.ts: cleanupNullTasksForTaskList$ (hidden dispatch via
  tagService.updateTag inside tap)
- reminder-countdown.effects.ts: reminderCountdownBanner$ (banner flash)
- voice-reminder.effects.ts: dominaMode$ (unwanted TTS)
- task-ui.effects.ts: timeEstimateExceeded$ and
  timeEstimateExceededDismissBanner$ (unwanted notifications/banners)

Also enable the existing require-hydration-guard ESLint rule in
eslint.config.js (scoped to *.effects.ts files) to prevent future
regressions. The rule was already written but never wired into the
linting pipeline.

https://claude.ai/code/session_01WcAdr12nsvLdAjubLx1ZLf

* fix: address PR review feedback for selector effects guards

- Fix require-hydration-guard.spec.js for ESLint v9 (parserOptions → languageOptions)
- Upgrade require-hydration-guard severity from warn to error
- Restore require-entity-registry rule (also lost in ESLint v9 migration)
- Remove unrelated package-lock.json noise

https://claude.ai/code/session_01WcAdr12nsvLdAjubLx1ZLf

---------

Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
Johannes Millan 2026-02-08 15:18:21 +01:00 committed by GitHub
parent ba09f2c440
commit 19796204f3
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 31 additions and 1 deletions

View file

@ -5,7 +5,7 @@ const { RuleTester } = require('eslint');
const rule = require('./require-hydration-guard');
const ruleTester = new RuleTester({
parserOptions: {
languageOptions: {
ecmaVersion: 2020,
sourceType: 'module',
},

View file

@ -3,6 +3,7 @@ const tseslint = require('typescript-eslint');
const angular = require('angular-eslint');
const prettierRecommended = require('eslint-plugin-prettier/recommended');
const preferArrow = require('eslint-plugin-prefer-arrow');
const localRules = require('eslint-plugin-local-rules');
module.exports = tseslint.config(
// Global ignores
@ -115,6 +116,17 @@ module.exports = tseslint.config(
'@typescript-eslint/no-wrapper-object-types': 'error',
},
},
// NgRx effects files - require hydration guards on selector-based effects
{
files: ['**/*.effects.ts'],
plugins: {
'local-rules': localRules,
},
rules: {
'local-rules/require-hydration-guard': 'error',
'local-rules/require-entity-registry': 'warn',
},
},
// HTML files
{
files: ['**/*.html'],

View file

@ -28,6 +28,7 @@ import { Router } from '@angular/router';
import { DataInitStateService } from '../../../core/data-init/data-init-state.service';
import { TaskSharedActions } from '../../../root-store/meta/task-shared.actions';
import { Log } from '../../../core/log';
import { skipWhileApplyingRemoteOps } from '../../../util/skip-during-sync.operator';
const UPDATE_PERCENTAGE_INTERVAL = 250;
// since the reminder modal doesn't show instantly we adjust a little for that
@ -44,10 +45,15 @@ export class ReminderCountdownEffects {
private _projectService = inject(ProjectService);
private _router = inject(Router);
/**
* SAFETY: Guarded with skipWhileApplyingRemoteOps() to prevent banner
* flashing during sync when selectAllTasksWithReminder changes.
*/
reminderCountdownBanner$ = createEffect(
() =>
this._dataInitStateService.isAllDataLoadedInitially$.pipe(
concatMap(() => this._store.select(selectReminderConfig)),
skipWhileApplyingRemoteOps(),
switchMap((reminderCfg) =>
reminderCfg.isCountdownBannerEnabled
? combineLatest([

View file

@ -32,6 +32,7 @@ import { selectAllTasksDueToday } from '../../planner/store/planner.selectors';
import { TaskSharedActions } from '../../../root-store/meta/task-shared.actions';
import { Log } from '../../../core/log';
import { skipDuringSyncWindow } from '../../../util/skip-during-sync-window.operator';
import { skipWhileApplyingRemoteOps } from '../../../util/skip-during-sync.operator';
import { alertDialog, confirmDialog } from '../../../util/native-dialogs';
@Injectable()
@ -112,9 +113,15 @@ export class TagEffects {
{ dispatch: false },
);
/**
* SAFETY: Guarded with skipWhileApplyingRemoteOps() because this effect
* dispatches via tagService.updateTag() inside tap(), bypassing dispatch:false.
* Without the guard, intermediate sync states could trigger false null-task detection.
*/
cleanupNullTasksForTaskList$: Observable<unknown> = createEffect(
() =>
this._workContextService.activeWorkContextTypeAndId$.pipe(
skipWhileApplyingRemoteOps(),
filter(({ activeType }) => activeType === WorkContextType.TAG),
switchMap(({ activeType, activeId }) =>
this._workContextService.mainListTasks$.pipe(

View file

@ -33,6 +33,7 @@ import { EMPTY } from 'rxjs';
import { selectProjectById } from '../../project/store/project.selectors';
import { Router } from '@angular/router';
import { NavigateToTaskService } from '../../../core-ui/navigate-to-task/navigate-to-task.service';
import { skipWhileApplyingRemoteOps } from '../../../util/skip-during-sync.operator';
@Injectable()
export class TaskUiEffects {
@ -121,6 +122,7 @@ export class TaskUiEffects {
timeEstimateExceeded$ = createEffect(
() =>
this._store$.pipe(select(selectConfigFeatureState)).pipe(
skipWhileApplyingRemoteOps(),
switchMap((globalCfg) =>
globalCfg && globalCfg.timeTracking.isNotifyWhenTimeEstimateExceeded
? // reset whenever the current taskId changes (but no the task data, which is polled afterwards)
@ -153,6 +155,7 @@ export class TaskUiEffects {
timeEstimateExceededDismissBanner$ = createEffect(
() =>
this._store$.pipe(select(selectConfigFeatureState)).pipe(
skipWhileApplyingRemoteOps(),
switchMap((globalCfg) =>
globalCfg && globalCfg.timeTracking.isNotifyWhenTimeEstimateExceeded
? this._bannerService.activeBanner$.pipe(

View file

@ -7,6 +7,7 @@ import { Store } from '@ngrx/store';
import { selectIsDominaModeConfig } from '../../config/store/global-config.reducer';
import { selectCurrentTask } from '../../tasks/store/task.selectors';
import { speak } from '../../../util/speak';
import { skipWhileApplyingRemoteOps } from '../../../util/skip-during-sync.operator';
@Injectable()
export class VoiceReminderEffects {
@ -16,6 +17,7 @@ export class VoiceReminderEffects {
dominaMode$: Observable<unknown> = createEffect(
() =>
this._store$.select(selectIsDominaModeConfig).pipe(
skipWhileApplyingRemoteOps(),
distinctUntilChanged(),
switchMap((cfg) =>
cfg.isEnabled && cfg.voice