Clarify/improve existing behavior in component-store initState method

[eLeontev]

Description:

Displaying of ${this.constructor.name} has not been initialized yet. Please make sure it is initialized before updating/getting. looks incorrect for multiple component-stores and possible for store + component-store integration in some cases.

Usually call of some store action in subscriber callback is bad practice, but in some cases it could be chain of calls where subscription from one store triggers initialization of another component-store.

[ ] Regression (a behavior that used to work and stopped working in a new release)
[ * ] Bug report  
[ ] Feature request
[ ] Documentation issue or request

What is the current behavior?

Initialized component-store actions throw the same error: ${this.constructor.name} has not been initialized yet. Please make sure it is initialized before updating/getting.

Expected behavior:

Actions of initialized store should work as expected.
Error should not be displayed.

Minimal reproduction of the problem with instructions:

native issue reproduction

class Store extends ComponentStore<{ shouldInitDerivedStore: boolean }> {
    constructor() {
        super({ shouldInitDerivedStore: false });
    }

    initDerivedStore = this.updater((state) => ({
        shouldInitDerivedStore: !state.shouldInitDerivedStore,
    }));
}

class DerivedStore extends ComponentStore<{ anyField: boolean }> {
    constructor() {
        super({ anyField: false });
    }

    anyAction = this.updater(() => ({ anyField: true }));
}

const store = new Store();
const initDerivedStoreCallback = () => {
    const derivedStore = new DerivedStore();
    derivedStore.anyAction(); // `DerivedStore has not been initialized yet. Please make sure it is initialized before updating/getting.`
};

store.state$
    .pipe(filter(({ shouldInitDerivedStore }) => shouldInitDerivedStore))
    .subscribe(initDerivedStoreCallback);

store.initDerivedStore();

Other information:

For practice usage in angular world:
Angular supports performance improvements like changeDetectionStrategy: OnPush. It means user need to handle view synchronisation manually in case if changes comes outside of angular word (any action which does not trigger change detection).
For these cases angular provides two options to update view:

• marckForCheck marks all components tree modified to the root entry to validate them on the next change detection call
• detectChanges runs change detection directly for the affected component.
  Usage of both methods depends on the used libraries and performance criteria.

If we have two components with different store instances we could not make them dependent if parent component trigger detectChanges inside the callback. In other words children component with its own store instance cannot be created from the subscription result of the parent component like next:

// in parent component
store.select(shouldDisplayChildSelector).subscribe((shouldDisplayChild) => {
  this.shouldDisplayChild = shouldDisplayChild;
  this.ref.detectChanges(); // this call means initialisation of child component performs before `queueScheduler` of rxjs will call all its actions
})

<!--parent component-->
<child-component-with-own-store *ngIf="shouldDisplayChild"><child-component-with-own-store />

Reason:

Looks like it is regression after this PR #2606

Possible solutions if expected behaviour will be confirmed

1. Simple way: refactor this method: https://github.com/ngrx/platform/blob/master/modules/component-store/src/component-store.ts#L149
   

to this way

   /**
     * Reason: staying isInitialized flag inside callback looks like code issue
     *
     * Description: staying the flag in the callback means we need to perform side effects (actions queue) before to init the store
     * even if user has already initialized it
     *
     * Fix details: once user init state in constructor or #setState the store will be initialized
     */
    private initState(state: T): void {
        this.isInitialized = true; // the fix

        scheduled([state], queueScheduler).subscribe((s) => {
            this.stateSubject$.next(s);
        });
    }

2. Complex way: stop use singleton queueScheduler in each component-stores instance and replace to multiple instances:

class ComponentStore {
  constructor() {
    this.queueScheduler = new queueScheduler()
  }
}

workarounds for angular:

1. if the reason is detectChanges call and it's correct for the application to postpone to call changeDetection. Simple replace its call to markForCheck
2. if the reason in subscriber. You can try to use https://rxjs.dev/api/operators/observeOn (looks like a hack)

this.observable$
  .pipe(observeOn(queueScheduler)) // this postpone to call subscribe till all actions in queue will be called (#initState/#setState are placed in this scheduler)
  .subscriber(callback)

3. as a hack solution - add any delay

[eLeontev]

I could prepare the fix PR if the expected behavior will be confirmed.

[ghostlytalamaur]

It would be great to use new instance of queueScheduler per component store. It will solve problem with recursive updates. Also it solves problem when store is created inside queueScheduler executing task and store might be uninitialized after instantiating altough initialState is passed through constructor.

[evantrimboli]

I'm facing a similar issue, during component initialization I conditionally load some data based on whether there's a parameter in the route. I want to set the loading state in my store, however by the time I make that call, the store is yet to be initialized. The code looks something like this:

interface Data {
    readonly loading: boolean
}

class MyStore extends ComponentStore<Data> {
    constructor() {
        super({
            loading: false
        })
    }

    loadId(id) {
       // Can't set state here, not initialized.
        this.setState({
            loading: true
        })
        // Remote call here
        return of({})
    }
}

@Component({
  selector: 'app-my-component',
  providers: [MyStore],
  changeDetection: ChangeDetectionStrategy.OnPush
})
export class MyComponent {
  constructor(
    route: ActivatedRoute,
    private readonly store: MyStore
  ) {
    const loader$ = route.params.pipe(
      map((p) => ({
        routeId: getRouteId('foo', p)
      })),
      switchMap(({ routeId }) => {
        if (route !== null) {
          return this.store.loadById(routeId);
        } else {
          return of(false);
        }
      }),
      shareReplay(1)
    );
  // Other stuff
  }
}

Would appreciate any advice.

[NachmanBerkowitz]

I am having a similar issue. My component store is initialized in the constructor, yet I get a not-initialized error. I only get this error if I start the app on the view that has the component store (i.e. refresh the browser on that view), not if I navigate there from within the app. Most probably because there is a lot qued on app initialization.

[Klapik]

Any news?

[monkeycatdog]

I have the same issue
Seems like the initialisation is async. weird.

[rovshenn]

Same issue here, using component store in a modal, requires us to provide the store in root, which raises above error.

[yurakhomitsky]

I'm facing a similar issue, during component initialization I conditionally load some data based on whether there's a parameter in the route. I want to set the loading state in my store, however by the time I make that call, the store is yet to be initialized. The code looks something like this:

interface Data {
    readonly loading: boolean
}

class MyStore extends ComponentStore<Data> {
    constructor() {
        super({
            loading: false
        })
    }

    loadId(id) {
       // Can't set state here, not initialized.
        this.setState({
            loading: true
        })
        // Remote call here
        return of({})
    }
}

@Component({
  selector: 'app-my-component',
  providers: [MyStore],
  changeDetection: ChangeDetectionStrategy.OnPush
})
export class MyComponent {
  constructor(
    route: ActivatedRoute,
    private readonly store: MyStore
  ) {
    const loader$ = route.params.pipe(
      map((p) => ({
        routeId: getRouteId('foo', p)
      })),
      switchMap(({ routeId }) => {
        if (route !== null) {
          return this.store.loadById(routeId);
        } else {
          return of(false);
        }
      }),
      shareReplay(1)
    );
  // Other stuff
  }
}

Would appreciate any advice.

Having the same issue in the tap setLoadingStatus

		super(initialState);
		this.getEquipment$(this.store.select(routerSelectors.getRouteWellId));

	private getEquipment$ = this.effect((wellId$: Observable<number>) => {
		return wellId$.pipe(
			tap(() => this.setLoadingStatus()),
			switchMap((wellId: number) =>
				this.dataService.getWellEquipment(wellId).pipe(
					tapResponse(
						(equipment: WellEquipment[]) => {
							this.setStatusByResponse(equipment);
							this.setEquipment(equipment);
						},
						() => {
							this.setErrorStatus();
						}
					)
				)
			)
		);
	});

	private setLoadingStatus(): void {
		this.patchState({ status: ActionStatusEnum.LOADING });
	}

[yurakhomitsky]

I am having a similar issue. My component store is initialized in the constructor, yet I get a not-initialized error. I only get this error if I start the app on the view that has the component store (i.e. refresh the browser on that view), not if I navigate there from within the app. Most probably because there is a lot qued on app initialization.

I was getting the same issue

I fixed it like this