bucket.set mutates irrelevant state key in the valueStream

[DmitryEfimenko]

Google search terms

"@extend-chrome/storage" bucket.set mutates irrelevant state key in the valueStream

Describe the bug

When using bucket.set method, and updating a particular property, the bucket.valueStream emits a mutated version of the whole state so referential equality does not work. See example:

interface Person {
  name: string;
}

interface Address {
  country: string;
}

interface State {
  person: Person;
  address: Address;
}

const bucket = getBucket<State>('state');

const person$ = bucket.valueStream.pipe(
  map(x => x.person),
  distinctUntilChanged()
);

const address$ = bucket.valueStream.pipe(
  map(x => x.address),
  distinctUntilChanged()
);

person$.subscribe(x => { console.log('person changed', x) });
address$.subscribe(x => { console.log('address changed', x) });

function updateAddress(country: string) {
  bucket.set(prev => {
    const updatedAddress = { ...prev.address, country };
    const updatedState = { ...prev, address: updatedAddress };
    return updatedState;
  })
}

updateAddress('France');
// observe console also emitting "person changed"

How do we reproduce?

Use the code from the example above. Unfortunately, StackBlitz does not work with this lib

Expected behavior

The state property that was not updated via the set method should not get mutated

Actual behavior

A state property that I didn't explicitly update got mutated.

Screenshots

Please complete the following information:

• Library Version: 1.5.0
• Operating System: Windows
• Browser: Chrome 97
• Node Version: 17

Additional context

[jacksteamdev]

The value stream works by retrieving all values from storage (no cache has been implemented), thus the two objects are not strictly equal (which is how distinctUntilChanged works). You might consider using a custom comparator to do a deep equals check, or maybe distinctUntilKeyChanged for better performance.

[DmitryEfimenko]

Unfortunately, that's not quite an option since the object I put in is an array. There's no key to check against and doing a deep check doesn't make sense either since it would require checking if each item in the array has changed in some way.

I came up with a workaround - an abstraction that creates a separate bucket for each key in the State object and it works for me, but I feel like this use-case should be handled by the library out of the box. Here's the abstraction:

import { Bucket, getBucket } from '@extend-chrome/storage';
import { ReplaySubject } from 'rxjs';
import { distinctUntilChanged, map, pluck, shareReplay, switchMap, tap } from 'rxjs/operators';

type Updater<T, K extends keyof T> = (prev: T[K]) => T[K];

export class BrowserStorage<T extends Record<string, any>> {
  private buckets: Record<keyof T, Bucket<any>> = {} as any;
  private bucketName: string;
  private defaults: T;

  private initStore$ = new ReplaySubject(1);

  constructor(bucketName: string, defaults: T) {
    this.bucketName = bucketName;
    this.defaults = defaults;
    // init a separate bucket for each key in the state as a workaround
    // for https://github.com/extend-chrome/storage/issues/18
    Object.keys(defaults).forEach((key: keyof T) => {
      this.buckets[key] = getBucket(this.getPropKey(key));
    });
    this.initStore$.next();
  }

  selectKey<K extends keyof T>(key: K) {
    return this.initStore$.pipe(
      switchMap(() => this.getBucket(key).valueStream),
      pluck(key),
      map(val => val ?? this.defaults[key]),
      distinctUntilChanged(),
      shareReplay(1)
    );
  }

  setKey<K extends keyof T>(
    key: K,
    updaterOrVal: T[K] | Updater<T, K>
  ) {
    this.getBucket(key).set(state => {
      const prev = state[key] ?? this.defaults[key];
      const updated = typeof updaterOrVal === 'function'
        ? (updaterOrVal as Updater<T, K>)(prev)
        : updaterOrVal;
      return { ...state, [key]: updated };
    });
  }

  private getBucket<K extends keyof T>(key: K) {
    return this.buckets[key] as Bucket<T>;
  }

  private getPropKey(key: keyof T) {
    return `${this.bucketName}-${key}`;
  }
}