Updating list upon ionic storage set() completion

Sorry in advance for the the lengthy post.

I am still pretty new to the PWA world and am having trouble getting values added to Storage to show up upon return to my customer list

My Service:

import { Injectable } from '@angular/core';
import { Storage } from '@ionic/storage';
import { Customer } from './models/customer';
import { StorageConstants } from './storageConstants';
import { Observable } from 'rxjs/Observable';
import { from } from 'rxjs';
import { map, tap } from 'rxjs/operators';

@Injectable({
  providedIn: 'root'
})
export class CustomersService {

  constructor(private storage: Storage) { }

  getCustomers() : Observable<Customer[]>{
    return from(this.storage.get(StorageConstants.customers)).pipe(
      map((customers) => {
        return customers ? JSON.parse(customers).map(
          (customer: Customer) => new Customer(customer)) : []
      })
    )
  }

  addCustomer(customer: Customer) {
    let customers_update: Customer[];
    return this.getCustomers().pipe(
      tap(
        (customers : Customer[]) => {
          customers_update = customers;
          customers_update.push(customer);
          this.storage.set(
            StorageConstants.customers, JSON.stringify(customers_update));
        }
      )
    );
  }
}

My Form component

import { Component, ViewChild, OnInit } from '@angular/core';
import { Validators, FormBuilder, FormGroup } from '@angular/forms';
import { CustomersService } from '../customers.service';
import { Customer } from '../models/customer';
import { Router } from '@angular/router';

@Component({
  selector: 'app-customer-form',
  templateUrl: './customer-form.page.html',
  styleUrls: ['./customer-form.page.scss'],
})
export class CustomerFormPage implements OnInit{
  @ViewChild('customerForm', {static: false}) formValues;

  private  addCustomer: FormGroup;
  constructor(
    private formBuilder: FormBuilder,
    private customersService: CustomersService,
    private router: Router
  ) {
    
    this.addCustomer = this.formBuilder.group(
      {
        name: ['', Validators.required],
        phone: ['', Validators.required],
        email: ['']
      }
    );
   }

   ngOnInit() {}

  onSubmit() {
      const newCustomer: Customer = new Customer(
        {
          name: this.addCustomer.value.name,
          phone: this.addCustomer.value.phone,
          email: this.addCustomer.value.email
        }
      );

      this.customersService.addCustomer(newCustomer).subscribe(
        {
          complete: () => this.router.navigate(['tabs', 'customers'])
        }
      );
  }

}

My list component

import { Component, OnInit } from '@angular/core';
import { CustomersService } from '../customers.service';
import { Customer } from '../models/customer';
import { NavController } from '@ionic/angular';
import { Observable } from 'rxjs';

@Component({
  selector: 'customers',
  templateUrl: 'customers.page.html',
  styleUrls: ['customers.page.scss']
})
export class CustomersPage implements OnInit{

  private customers: Observable<Customer[]>;

  constructor(private customersService: CustomersService) {}

  ngOnInit(): any {
    this.customers = this.customersService.getCustomers();
  }
}

List html

<ion-content>
  <div *ngFor="let customer of customers | async">
    <h2>{{customer.name}} {{customer.phone}}</h2>
  </div>
</ion-content>

<ion-fab vertical="bottom" horizontal="end" slot="fixed">
  <ion-fab-button [routerLink]="['add-customer']">
    <ion-icon name="add"></ion-icon>
  </ion-fab-button>
</ion-fab>

Adding a customer via the addCustomer method in the service adds to Storage, but the list in the component is only updated upon refresh. I looked around and tried a lot of solutions, but I think I am missing a fundamental understanding about how these pieces are working together.

Any help is greatly appreciated.

Length is cool, especially when the post is as well-written and well-formatted as this one is. You are under absolutely no compulsion to take my opinion as anything other than the opinion of a really opinionated opinion-haver, and more than likely somebody else will be along shortly to offer a different one.

That being said, I recognize in your code several things that I used to do when I was starting out writing webapps, and no longer do any more for various reasons. I can expand more on why if desired, but:

I spent a lot of time writing C++, and putting as much “intelligence” into data-bearing objects is pretty idiomatic for OOP code. JavaScript is not an OO language. It pretends to be one, but you will be happier in the long run if you refuse to believe it. I now completely swear off “smart” data-bearing objects in JavaScript in favor of TypeScript interfaces (which are just POJOs), and choose to put all the “smarts” in wranglers. One major reason is because we get to avoid all of this error-prone tedium:

.pipe(
      map((customers) => {
        return customers ? JSON.parse(customers).map(
          (customer: Customer) => new Customer(customer)) : []
      })
    )

We no longer have to worry about sprinkling magic Customer constructor pixie dust on things, a problem our toolchain unfortunately can’t help us avoid because JavaScript.

So I would convert Customer to an interface and put whatever intelligence is currently in it into CustomersService instead.

This looks like a neat feature, but one showstopper problem with it is that you can’t override it elsewhere. This makes mocking it out for testing virtually impossible. So I don’t use it, instead just declaring services in the provider stanza of my app module.

Now we get to the main event:

Never use Storage to communicate within runs of an app. Use it only to talk to the future and listen to the past.

Your situation here is precisely why I adopted this rule. Yes, it’s possible to implement the design you’ve chosen by judiciously waiting off the Promise returned by storage.set. However, as you’ve discovered, it’s a PITA to do. It also introduces a needless performance bottleneck. So what I would do instead is this:

import {clone} from "lodash-es";
export class CustomersService {
  private customers$ = new BehaviorSubject<Customer[]>([]);
  constructor(private storage: Storage) {
    // this is the only time we read from storage:
    // once at app startup to listen to the past
    storage.ready()
      .then(() => storage.get(StorageConstants.customers))
      .then((customers) => {
        if (customers) { this.customers$.next(customers); }
      });
  }
  getCustomers(): Observable<Customer[]> {
    return this.customers$;
  }
  addCustomer(customer: Customer): void {
    // this clone may be avoided in some circumstances, but exists because
    // it's not easy to detect changes to deep object references in JavaScript
    let newCustomers = clone(this.customers$.value);
    newCustomers.push(customer);
    // the in-app communication is done entirely within CustomersService
    this.customers$.next(newCustomers);
    // here we speak to the future, and don't care when it's ready to be heard
    this.storage.set(StorageConstants.customers, newCustomers);
  }
}
2 Likes

I will explore the BehaviorSubject. Thanks for your response.

lol, I like development opinions, I try to have as many of my own as possible. I am interested in the idea of removing a class in favor of an interface. I’ll have to read up a bit. That’s usually how these things end… More research for me to do :confused:

rapropros in an expert
I’m a simple user and he give me the pleasure to share :wink:
The question is why not use ngrx ? it let you manage a store with action/reducer process.

I started trying to use it once, and decided it was too elaborate for the benefit I saw out of it. Maybe at some point in the future I’ll try it again. I think if one is already familiar with Flux or Redux, maybe it will feel more natural, but I don’t fall into that category.

I had a similar feeling working with it as I did working with Hibernate when doing Java stuff - in the end I decided I am more comfortable just writing SQL than dealing with ORMs, and sort of feel similarly about RxJS versus ngrx.

So, in the end, this opinionated opinion-haver doesn’t have a strong opinion on ngrx - just hasn’t felt a need to get more familiar with it yet.

Yep understand (by my frenchy langage :wink:

Worked like a charm. Thank you.

How would you access customer data from the CustomerListComponent?
For example, if you want to sort on Firstname or Lasrname?

Generally speaking, my first instinct would be to carry an array of customers in the component. There are several options of what to do when sorting or filtering is in place and the upstream “universe” (all customers) changes: apply our sort / filter to the new universe, ditch it and overwrite entirely, wait until filters are off to apply upstream changes. Here’s a simple example to get you started:

@UntilDestroy()
export class CustomerListComponent {
  customers: Customer[] = [];
  customerSorter?: (a: Customer, b: Customer) => number;

  constructor(private customerer: CustomerService) {
    this.customerer.getCustomers().pipe(untilDestroyed(this)).subscribe(customers = {
      this.customers = customers;
      this.sortCustomers();
    });
  }

  sortCustomers(): void {
    if (this.customerSorter) {
     this.customers.sort(this.customerSorter);
    }
  }

  sortByLastName(): void {
    this.customerSorter = (a: Customer, b: Customer) => {
      return a.lastName.localeCompare(b.lastName);
    }; 
    this.sortCustomers();
  }

  sortByFirstName(): void {
    this.customerSorter = (a: Customer, b: Customer) => {
      return a.firstName.localeCompare(b.firstName);
    };
    this.sortCustomers();
  }
}
1 Like

So we end up with two copies in memory of the customer list !
What if I have a large list ? I was trying to keep a single copy in the provider.
Also, this.customers = customers, will it give the address of the list in the provider or a “copy” the list ?

Unless you’ve profiled the app and found that memory needed to store the customer list is a noticeable concern for app performance, I would suggest a working hypothesis that this would be somewhere around #724 on my list of stuff to worry about.

People watch videos and play castle tower defense games and edit photos of themselves to look like they are anime characters on the devices your app is likely to be running on. Those sorts of activities absolutely dwarf the storage (and CPU) requirements needed for slinging typical textual data around, even a lot of fairly extensive textual data. If your Customer does have super-heavy images or videos included in it, keep reading.

Yeah, this is a great question and goes to the heart of just how pathetic JavaScript’s fake “OO” really is. I think I’ve been working in JavaScript (kicking and screaming daily) for almost a decade now and still don’t really have a solid, concise recommendation for how to work with it on this topic.

I’m going to assume you’ve got some familiarity with C or another C-like language, due to your use of the term “address of”. The short answer to your question is “the address of the list”, in the sense that although everything in JavaScript is technically pass-by-value, the “value” of an object (or an array, which is a special kind of object, even though you generally shouldn’t think of them that way) is effectively a pointer in C terms, with all the baggage that goes with that.

Entire libraries like Immutable.js and Redux have sprung up attempting to deal with “when do I want a pointer copy, a shallow clone, or a deep clone?”. I have no experience yet with any of them.

But yes, you have to worry about this. When you call something like sort that modifies an array in place, everybody else holding a reference to that array gets affected. If that’s a problem, then you can start cloning things or defining ownership rules.

Again, though, textual data is super, super lightweight in today’s world. You are going to have much more urgent performance (and UI) concerns figuring out how to let users interact with lots of customers way before you have to worry about how many copies of the list you have in memory. If a Customer has heavy fields like a movie or high-res image, worry about those, but unless you’re blindly deep-cloning everything, those heavy bits will effectively be shared by reference.

1 Like

Once again, efficient answer. Thanks a lot. I’ll try worrying about stuff that matters. And i’ll keep an eye out for potential side effects of working/modifying an object taken from a provider.

Why not directly expose “public customer$”? Is it just to prevent some other piece of code to set it with “customer$.next(x)” or something other?

Exactly. There is (or used to be in old versions of RxJS, at least) an asObservable method of Subject that does the same thing explicitly, but once I noticed that even in the Angular source they don’t bother to call it, but instead just return the Subject out of a method declared to return an Observable in order to achieve that effect, I started doing that instead.

1 Like

Why “clone” the existing customers$ ? Anything wrong with just
customers$.next( customers$.value.push(customer) );

I understand that when subscribing to an observable, you get notified when it changes. But when the observable is a function like getCustomers(), what’s the underlying mechanic to detect that the ‘function’ changed ??? I have it all setup and it works fine, but I just can’t grasp how it does it.

Two primary reasons:

Change detection

$ ts-node
> import {BehaviorSubject} from "rxjs";
> import {distinctUntilChanged} from "rxjs/operators";
> import {clone} from "lodash";
> let s$ = new BehaviorSubject<number[]>([]);
> s$.subscribe(nv => console.log("raw " + JSON.stringify(nv)));
raw []
> s$.pipe(distinctUntilChanged()).subscribe(nv => console.log("duc " + JSON.stringify(nv)));
duc []
> s$.next([1]);
raw [1]
duc [1]

happy so far

> let nv = clone(s$.value); nv.push(2); s$.next(nv);
raw [1,2]
duc [1,2]

still cruising, but here comes the patch of black ice

> let xv = s$.value; xv.push(3); s$.next(xv);
raw [1,2,3]

…and crickets from the duc subscription, because it can’t recognize anything as “changed” except the top-level reference.

Aliasing

$ ts-node
> import {BehaviorSubject} from "rxjs";
> let s$ = new BehaviorSubject<number[]>([]);
> let elsewhere: number[] = [];
> let sub1 = s$.subscribe(nv => elsewhere = nv);
> s$.next([1]);
> elsewhere;
[ 1 ]

So far, so good, but now we have aliased elsewhere to the array buried deep within s$.

> sub1.unsubscribe();
> elsewhere === s$.value;
true

ruh roh

> let v = s$.value;
> v.push(2);
> elsewhere;
[ 1, 2 ]

Well, customers$.next is expecting an array, but Array.push returns the array length (this is just an excuse for me to dunk on JavaScript some more…please don’t take it personally…it was totally clear what you meant). Hopefully the underlying question is addressed sufficiently above.

1 Like

The observable in question isn’t really the function getCustomers(), but the thing that function returns. Let’s say there is an animal shelter in your neighborhood called “Furry Friends”. You go there and adopt a cat that the shelter employees have named “Kujira”, because it’s grey and fairly chonky. You put the cat on a diet, it loses weight, you decide to call it “Dorado” instead. This does not rename the animal shelter.

Ok so is it to say that since getCustomers() returns a reference to customers$, subscribing to the function is equivalent to subscribing to the Observable, with the exception that subscribers cannot do getCustomers().next(x) ?

Yes, that’s a fair way of putting it, except we have to weaken “cannot” down to “cannot without tsc yelling at them”, because once it gets transpiled down to JavaScript at runtime, there’s no more access control.

1 Like