Why is my state's @Selector() returning both the previous and current data in one call?

Question

The shape of my state looks like this

@State<ProductDataModel>({
    name: 'productDataState',
    defaults:{
        categories: [],
        currentProduct: {
            productId : '',
            name : '',
            description : '',
            instructionKey : '',
            models : []
        }
    }
})

I created a @Selector() for returning the currentProduct property which looks like this.

@Selector()
    static fetchCurrentProduct(state:ProductDataModel){
        console.log(state.currentProduct);
        return state.currentProduct;
    }

The console.log() here returns 2 currentProduct objects when my component loads which inside my component I use the @Select(state.data) VariableName$ way of calling the data. When console.log()ing the @Select() variable inside the component in the ngOnInit() hook the number of responses it gets back goes up by one each instance.

I created an @Action() for setting the currentProduct property by checking inside the state for the product, if it exists it plugs it patches the currentProduct property. If it doesn't exists it makes an api request to my NestJS app and adds it to the proper category and sets it as the current product.

export class FetchProduct{
    static readonly type='[Product Data] Fetch Product';
    constructor(public category: string, public product: string){console.log(product);}
}

@Action(FetchProduct)
    public fetchProduct(ctx:StateContext<ProductDataModel>, {category, product}: FetchProduct){

        const currentState = ctx.getState();//gets current state

        //fetches the category from the state
        const categoryMatch = currentState.categories.find(cat => cat.productCategoryId === category);

        //search to see if the product already exists
        const productMatch = categoryMatch.products.find(p => p.productId === product); 

        //if the product exists set it as the current product
        if(productMatch){ ctx.patchState({currentProduct: productMatch}); }


        //if the product doesn't exist fetch it from the backend app
        else{

            //fetchProductData() makes api request and returns an Observable
            return this.dataService.fetchProductData(category, product).pipe(tap(result =>{

                //empty variable to store mutated category data
                const newCategoryList: ProductCategoryModel[] = [];

                //clone of original category data
                const categoryList: ProductCategoryModel[] = [...currentState.categories];

                //creates an object out of the returned product
                const newProduct: ProductDataItem ={
                    productId: result.productId,
                    name: result.name,
                    description: result.description,
                    instructionKey: result.instructionKey,
                    models: result.models
                };

                //create updated category data
                categoryList.forEach(a=>{

                    //adds product to respective category and pushes to empty variable
                    if(a.productCategoryId === category){
                        const newItem: ProductCategoryModel = {
                            productCategoryId: a.productCategoryId,
                            name: a.name,
                            description: a.description,
                            products: [...a.products, newProduct]
                        };

                        newCategoryList.push(newItem);
                    }

                    //pushes categories to variable that don't need mutating 
                    else{ newCategoryList.push(a); }
                });

                //updates the state
                ctx.patchState({categories: newCategoryList, currentProduct: newProduct});
            }));
        }
    }

I noticed that when I click on a product that has already been loaded into the state triggering this snippet

if(productMatch){ ctx.patchState({currentProduct: productMatch}); }

Everything works exactly as expected.

It seems to create a problem when the API request is added to the equation. However when console.log()ing through each part of my action everything is returned as expected. Logging the properties inside the constructor of my FetchProduct class only shows one call being triggered per click. The function in my service file that makes the API call looks like this.

public fetchProductData(category: string, product: string):Observable<ProductDataItem> /*ProductDataItem*/{
        const path = `${this.URL}/get-product/find-product?cattegory=${category}&product=${product}`;
        const data: Observable<ProductDataItem> = this.httpClient.get(path) as Observable<ProductDataItem>;

        data.subscribe(a=> console.log(a));
        return data;
    }

The console.log() here only returns one response per click as well. The way I call to the currentProduct property in my component looks like this.

@Select(ProductDataState.fetchCurrentProduct) Product$: Observable<ProductDataItem>;

Then I do this inside of the OnInit hook

this.Product$.subscribe(a=> console.log(a));

As I said before this shows multiple instances as well except the number of values it returns increases with each click.

I've gone as far as modifying my action like this

//set the currentProduct back to an empty object
 ctx.patchState({currentProduct: {
                    productId: '',
                    name: '',
                    description: '',
                    instructionKey: '',
                    models: []
                }});


                //adds product to category
                ctx.patchState({categories: [...newCategoryList]});

                //sets currentProduct from the object now saved inside the state
                ctx.patchState({
                    currentProduct: ctx.getState().categories.find(a=>a.productCategoryId === category)
                    .products.find(a=> a.productId === product)
                });

I tried this to see if would make a difference by updating it the same way it does when the product already exists but I still get the same results. Can anyone see what I'm doing wrong to create this behavior or does anybody know of anything under the hood with NGXS that I might be confusing some type of way?

Update

I moved the code that sets the currentProduct property to all empty values up to the first thing that gets done inside my FetchProduct action and I now get the correct data reflected in my component's Product$ observable, however there's still tons of values showing up in my console, none of my other Observables behave like this.

Answer 1

If I understand the desired behaviour correctly, something like this should work. The ngOnInit hook will be called once for this component so you want to wire up your subscription here, and allow it to update on each change - rebuilding the ModelSummaryData array each time a change is emitted. And as the other answers have noted, clean up the subscription when the component is destroyed.

export class ProductDashboardComponent implements OnInit, OnDestroy {

  @Select(ProductDataState.fetchCurrentProduct) Product$: Observable<ProductDataItem>;

  ModelSummaryData: ModelDataItemSummary[] = [];

  private destroyed$ = new Subject<void>();

  constructor(private dataService: ProductPageService, private store: Store) { }

   ngOnInit(): void { 
     this.initModelSummaryData();
   }

   ngOnDestroy(): void {
       this.destroyed$.next();
       this.destroyed$.complete();
   }  

   private initModelSummaryData(): void {
       this.Product$.pipe(
            tap(a => {
               // clear the summary data, so it's rebuild with the latest update
               this.ModelSummaryData = [];

               a.models.forEach(b => {
                   const item: ModelDataItemSummary = {
                       modelId: b.modelId,
                       name: b.name,
                       photo: b.photos.find(c=> c.cover === true).path
                   };
                  ModelSummaryData.push(item);
                });
            }),                
            takeUntil(this.destroyed$), // Clean up the subscription 
       ).subscribe();
    }
}

Answer 2

you have a memory leak, you need to clean up subscriptions when you destroy a component:

private productSub: Subscription;

ngOnDestroy() { this.productSub.unsubscribe(); }

private createModelSummaryData(): ModelDataItemSummary[]{
    const modelSummaryList: ModelDataItemSummary[] = [];

    this.productSub = this.Product$.subscribe(a=>{

        a.models.forEach(b=>{
            const item: ModelDataItemSummary = {
                modelId: b.modelId,
                name: b.name,
                photo: b.photos.find(c=> c.cover === true).path
            };

            modelSummaryList.push(item);

        });
   });

}

Answer 3

This is because when you switch between the products you are not unsubscribing to the old observable. Just unsubscribe from it. Please find the code below:

private createModelSummaryData(): ModelDataItemSummary[]{
    const productSubscription:Subscription;
    const modelSummaryList: ModelDataItemSummary[] = [];

    if (productSubscription) {
      productSubscription.unsubscribe();
    }
    productSubscription = this.Product$.subscribe(a=>{

        a.models.forEach(b=>{
            const item: ModelDataItemSummary = {
                modelId: b.modelId,
                name: b.name,
                photo: b.photos.find(c=> c.cover === true).path
            };

            modelSummaryList.push(item);

        });
   });

}