mirror of
https://github.com/johannesjo/super-productivity.git
synced 2026-07-17 16:37:43 +00:00
perf(work-context): collapse per-tick activeWorkContext$ churn with distinctUntilChanged (#8806)
selectActiveWorkContext re-runs every second while a task tracks time (selectTaskEntitiesInActiveProjects gets a new reference each tick), allocating a new WorkContext whose content is usually identical. That cascaded: activeWorkContextTTData$ tore down and re-subscribed the time-tracking state$ merge every second, and activeWorkContextTitle$ re-ran. Add a content-aware distinctUntilChanged (arrays compared by content, all other fields by reference — correct under NgRx) so no-op per-tick re-emissions collapse to one while any real change still emits. SPAP-18
This commit is contained in:
parent
cb6780933b
commit
9ae49a9e82
4 changed files with 258 additions and 2 deletions
|
|
@ -0,0 +1,95 @@
|
|||
import { isSameActiveWorkContext } from './is-same-active-work-context.util';
|
||||
import { WorkContext, WorkContextType } from './work-context.model';
|
||||
|
||||
const baseCtx = (): WorkContext =>
|
||||
({
|
||||
id: 'ctx1',
|
||||
type: WorkContextType.TAG,
|
||||
title: 'x',
|
||||
icon: null,
|
||||
routerLink: 'tag/ctx1',
|
||||
isEnableBacklog: true,
|
||||
theme: {},
|
||||
advancedCfg: {},
|
||||
taskIds: ['A', 'B'],
|
||||
backlogTaskIds: ['C'],
|
||||
noteIds: ['N'],
|
||||
}) as unknown as WorkContext;
|
||||
|
||||
describe('isSameActiveWorkContext', () => {
|
||||
it('returns true for the same reference', () => {
|
||||
const a = baseCtx();
|
||||
expect(isSameActiveWorkContext(a, a)).toBe(true);
|
||||
});
|
||||
|
||||
it('returns true for different-reference objects with identical content (incl. fresh taskIds array)', () => {
|
||||
const a = baseCtx();
|
||||
// Models a per-tick re-run: stable object refs (theme/advancedCfg) are
|
||||
// shared, arrays are freshly regenerated with the same values.
|
||||
const b = {
|
||||
...a,
|
||||
taskIds: [...a.taskIds],
|
||||
backlogTaskIds: [...(a.backlogTaskIds as string[])],
|
||||
noteIds: [...a.noteIds],
|
||||
} as unknown as WorkContext;
|
||||
expect(a).not.toBe(b);
|
||||
expect(a.taskIds).not.toBe(b.taskIds);
|
||||
expect(isSameActiveWorkContext(a, b)).toBe(true);
|
||||
});
|
||||
|
||||
it('returns false when taskIds content differs', () => {
|
||||
const a = baseCtx();
|
||||
const b = { ...a, taskIds: ['A', 'X'] } as unknown as WorkContext;
|
||||
expect(isSameActiveWorkContext(a, b)).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false when backlogTaskIds content differs', () => {
|
||||
const a = baseCtx();
|
||||
const b = { ...a, backlogTaskIds: ['Z'] } as unknown as WorkContext;
|
||||
expect(isSameActiveWorkContext(a, b)).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false when noteIds content differs', () => {
|
||||
const a = baseCtx();
|
||||
const b = { ...a, noteIds: ['N', 'M'] } as unknown as WorkContext;
|
||||
expect(isSameActiveWorkContext(a, b)).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false when title changes', () => {
|
||||
const a = baseCtx();
|
||||
const b = { ...a, title: 'y' } as unknown as WorkContext;
|
||||
expect(isSameActiveWorkContext(a, b)).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false when theme is a new object reference', () => {
|
||||
const a = baseCtx();
|
||||
const b = { ...a, theme: {} } as unknown as WorkContext;
|
||||
expect(a.theme).not.toBe(b.theme);
|
||||
expect(isSameActiveWorkContext(a, b)).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false when advancedCfg is a new reference', () => {
|
||||
const a = baseCtx();
|
||||
const b = { ...a, advancedCfg: {} } as unknown as WorkContext;
|
||||
expect(a.advancedCfg).not.toBe(b.advancedCfg);
|
||||
expect(isSameActiveWorkContext(a, b)).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false when isEnableBacklog changes', () => {
|
||||
const a = baseCtx();
|
||||
const b = { ...a, isEnableBacklog: false } as unknown as WorkContext;
|
||||
expect(isSameActiveWorkContext(a, b)).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false when icon changes', () => {
|
||||
const a = baseCtx();
|
||||
const b = { ...a, icon: 'star' } as unknown as WorkContext;
|
||||
expect(isSameActiveWorkContext(a, b)).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false when the key counts differ (extra key)', () => {
|
||||
const a = baseCtx();
|
||||
const b = { ...a, extraKey: 'boom' } as unknown as WorkContext;
|
||||
expect(isSameActiveWorkContext(a, b)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
|
@ -0,0 +1,42 @@
|
|||
import { WorkContext } from './work-context.model';
|
||||
import { fastArrayCompare } from '../../util/fast-array-compare';
|
||||
|
||||
/**
|
||||
* distinctUntilChanged comparator for activeWorkContext$. The active work
|
||||
* context selector re-runs every second while a task tracks time (task
|
||||
* entities get a new reference each tick), allocating a new WorkContext whose
|
||||
* content is usually identical. Returns true when two emissions are equivalent
|
||||
* for every field, so per-tick churn collapses to one emission while any real
|
||||
* change (context switch, task-membership change, theme/cfg edit, ...) still
|
||||
* emits. Arrays are compared by content (the selector regenerates them each
|
||||
* tick); all other fields (incl. nested theme/advancedCfg) are compared by
|
||||
* reference, which is correct under NgRx — a real change yields a new reference.
|
||||
*/
|
||||
export const isSameActiveWorkContext = (a: WorkContext, b: WorkContext): boolean => {
|
||||
if (a === b) {
|
||||
return true;
|
||||
}
|
||||
if (!a || !b) {
|
||||
return false;
|
||||
}
|
||||
const aKeys = Object.keys(a);
|
||||
const bKeys = Object.keys(b);
|
||||
if (aKeys.length !== bKeys.length) {
|
||||
return false;
|
||||
}
|
||||
for (const key of aKeys) {
|
||||
const av = (a as Record<string, unknown>)[key];
|
||||
const bv = (b as Record<string, unknown>)[key];
|
||||
if (av === bv) {
|
||||
continue;
|
||||
}
|
||||
if (Array.isArray(av) && Array.isArray(bv)) {
|
||||
if (!fastArrayCompare(av as unknown[], bv as unknown[])) {
|
||||
return false;
|
||||
}
|
||||
continue;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
return true;
|
||||
};
|
||||
|
|
@ -2,7 +2,7 @@ import { TestBed } from '@angular/core/testing';
|
|||
import { of } from 'rxjs';
|
||||
import { WorkContextService } from './work-context.service';
|
||||
import { TaskWithSubTasks } from '../tasks/task.model';
|
||||
import { provideMockStore } from '@ngrx/store/testing';
|
||||
import { MockStore, provideMockStore } from '@ngrx/store/testing';
|
||||
import { provideMockActions } from '@ngrx/effects/testing';
|
||||
import { Router } from '@angular/router';
|
||||
import { TranslateModule } from '@ngx-translate/core';
|
||||
|
|
@ -12,7 +12,9 @@ import { DateService } from '../../core/date/date.service';
|
|||
import { TimeTrackingService } from '../time-tracking/time-tracking.service';
|
||||
import { TaskArchiveService } from '../archive/task-archive.service';
|
||||
import { TODAY_TAG } from '../tag/tag.const';
|
||||
import { WorkContextType } from './work-context.model';
|
||||
import { WorkContext, WorkContextType } from './work-context.model';
|
||||
import { selectActiveWorkContext } from './store/work-context.selectors';
|
||||
import { allDataWasLoaded } from '../../root-store/meta/all-data-was-loaded.actions';
|
||||
|
||||
describe('WorkContextService - undoneTasks$ filtering', () => {
|
||||
let tagServiceMock: jasmine.SpyObj<TagService>;
|
||||
|
|
@ -525,3 +527,118 @@ describe('WorkContextService - getDoneTodayInArchive', () => {
|
|||
expect(result).toBe(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe('WorkContextService - activeWorkContext$ distinctUntilChanged', () => {
|
||||
let timeTrackingServiceMock: jasmine.SpyObj<TimeTrackingService>;
|
||||
let store: MockStore;
|
||||
let service: WorkContextService;
|
||||
|
||||
const ctx1 = (): WorkContext =>
|
||||
({
|
||||
id: TODAY_TAG.id,
|
||||
type: WorkContextType.TAG,
|
||||
title: 'x',
|
||||
icon: null,
|
||||
routerLink: 'tag/TODAY',
|
||||
theme: {},
|
||||
advancedCfg: {},
|
||||
taskIds: [],
|
||||
backlogTaskIds: [],
|
||||
noteIds: [],
|
||||
}) as unknown as WorkContext;
|
||||
|
||||
const CTX1 = ctx1();
|
||||
|
||||
beforeEach(() => {
|
||||
const tagServiceMock = jasmine.createSpyObj('TagService', ['getTagById$']);
|
||||
tagServiceMock.getTagById$.and.returnValue(of(TODAY_TAG));
|
||||
|
||||
const globalTrackingIntervalServiceMock = jasmine.createSpyObj(
|
||||
'GlobalTrackingIntervalService',
|
||||
[],
|
||||
{ todayDateStr$: of('2026-04-06') },
|
||||
);
|
||||
|
||||
const dateServiceMock = jasmine.createSpyObj('DateService', ['todayStr']);
|
||||
dateServiceMock.todayStr.and.returnValue('2026-04-06');
|
||||
|
||||
timeTrackingServiceMock = jasmine.createSpyObj('TimeTrackingService', [
|
||||
'getWorkStartEndForWorkContext$',
|
||||
]);
|
||||
timeTrackingServiceMock.getWorkStartEndForWorkContext$.and.returnValue(of({}));
|
||||
|
||||
const taskArchiveServiceMock = jasmine.createSpyObj('TaskArchiveService', [
|
||||
'loadYoung',
|
||||
]);
|
||||
taskArchiveServiceMock.loadYoung.and.returnValue(
|
||||
Promise.resolve({ ids: [], entities: {} }),
|
||||
);
|
||||
|
||||
TestBed.configureTestingModule({
|
||||
imports: [TranslateModule.forRoot()],
|
||||
providers: [
|
||||
provideMockStore({
|
||||
initialState: {
|
||||
workContext: { activeId: TODAY_TAG.id, activeType: 'TAG' },
|
||||
tag: { entities: {}, ids: [] },
|
||||
project: { entities: {}, ids: [] },
|
||||
task: { entities: {}, ids: [] },
|
||||
},
|
||||
}),
|
||||
// activeWorkContext$ is gated behind _afterDataLoadedOnce$, which only
|
||||
// fires after the allDataWasLoaded action.
|
||||
provideMockActions(() => of(allDataWasLoaded())),
|
||||
{ provide: Router, useValue: { events: of(), url: '/' } },
|
||||
{ provide: TagService, useValue: tagServiceMock },
|
||||
{
|
||||
provide: GlobalTrackingIntervalService,
|
||||
useValue: globalTrackingIntervalServiceMock,
|
||||
},
|
||||
{ provide: DateService, useValue: dateServiceMock },
|
||||
{ provide: TimeTrackingService, useValue: timeTrackingServiceMock },
|
||||
{ provide: TaskArchiveService, useValue: taskArchiveServiceMock },
|
||||
WorkContextService,
|
||||
],
|
||||
});
|
||||
|
||||
store = TestBed.inject(MockStore);
|
||||
store.overrideSelector(selectActiveWorkContext, CTX1);
|
||||
store.refreshState();
|
||||
|
||||
service = TestBed.inject(WorkContextService);
|
||||
});
|
||||
|
||||
it('collapses per-tick no-op re-emissions but still emits real changes', () => {
|
||||
const collected: WorkContext[] = [];
|
||||
const sub = service.activeWorkContext$.subscribe((v) => collected.push(v));
|
||||
// Subscribe to TTData$ so we can prove it does NOT re-subscribe on no-ops.
|
||||
const ttSub = service.activeWorkContextTTData$.subscribe();
|
||||
|
||||
expect(collected.length).toBe(1);
|
||||
expect(timeTrackingServiceMock.getWorkStartEndForWorkContext$.calls.count()).toBe(1);
|
||||
|
||||
// No-op: content-identical but new object reference (fresh taskIds array),
|
||||
// exactly what the selector produces every tracking tick.
|
||||
store.overrideSelector(selectActiveWorkContext, {
|
||||
...CTX1,
|
||||
taskIds: [...CTX1.taskIds],
|
||||
} as WorkContext);
|
||||
store.refreshState();
|
||||
|
||||
expect(collected.length).toBe(1);
|
||||
expect(timeTrackingServiceMock.getWorkStartEndForWorkContext$.calls.count()).toBe(1);
|
||||
|
||||
// Genuine change: different taskIds content -> must emit + re-subscribe TT.
|
||||
store.overrideSelector(selectActiveWorkContext, {
|
||||
...CTX1,
|
||||
taskIds: ['NEW'],
|
||||
} as WorkContext);
|
||||
store.refreshState();
|
||||
|
||||
expect(collected.length).toBe(2);
|
||||
expect(timeTrackingServiceMock.getWorkStartEndForWorkContext$.calls.count()).toBe(2);
|
||||
|
||||
ttSub.unsubscribe();
|
||||
sub.unsubscribe();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -58,6 +58,7 @@ import { selectNotesById } from '../note/store/note.reducer';
|
|||
import { TranslateService } from '@ngx-translate/core';
|
||||
import { T } from '../../t.const';
|
||||
import { fastArrayCompare } from '../../util/fast-array-compare';
|
||||
import { isSameActiveWorkContext } from './is-same-active-work-context.util';
|
||||
import { isShallowEqual } from '../../util/is-shallow-equal';
|
||||
import { distinctUntilChangedObject } from '../../util/distinct-until-changed-object';
|
||||
import { DateService } from 'src/app/core/date/date.service';
|
||||
|
|
@ -147,6 +148,7 @@ export class WorkContextService {
|
|||
|
||||
activeWorkContext$: Observable<WorkContext> = this._afterDataLoadedOnce$.pipe(
|
||||
switchMap(() => this._store$.select(selectActiveWorkContext)),
|
||||
distinctUntilChanged(isSameActiveWorkContext),
|
||||
shareReplay(1),
|
||||
);
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue