Noitidart / valtio-persist

MIT License
68 stars 4 forks source link

Whitelisted properties do not persist for newer valtio versions #7

Open dawsonbooth opened 1 year ago

dawsonbooth commented 1 year ago

valtio-persist is not persisting "whitelisted" properties specified with persistStrategies as documented. When the persistStrategies is set holistically to either PersistStrategy.SingleFile or PersistStrategy.MultiFile, it properly persists the entirety of the proxy object.

I tracked this down to the persistPath function which is simply not being called when a whitelisted property updates. I assume this is because the subscription fails.

I suspect it has to do with this valtio change in v1.7.1.

For the time being, we've downgraded to the version listed in this repo's package.json, v1.2.5, but it would be nice to be able to use this library on newer versions.

Noitidart commented 1 year ago

Thanks for sharing and digging! I have a new model I want to submit to this repo soon it might fix this. I've been using the new version for sometime, and going to take it to production by the end of this month. I'll keep you updated on how the new version fairs.

dawsonbooth commented 1 year ago

Sounds great, thanks for the quick response!

dawsonbooth commented 1 year ago

Hey there, just wondering if there's been any progress on this since the last post? :)

Noitidart commented 1 year ago

Hey @dawsonbooth - sorry about that, I haven't made it public yet. I can share the work for it though. It's a cool driver that can be used with anything on top. Valtio, redux or anything can connect to the underlying layer which handles reading/writing to disk. I called it createPersistoid, here's the work, its ugly, but wasnt meant for public eyes haha, so please bear with me. Would love if you want to roll something with this and jump start on the lib.

import AsyncLock from 'async-lock';

import retry from 'lib/retry';
import { addDebugBreadcrumb, captureSentry } from 'lib/sentry';

export type IStorageEngine = {
  // returns null if file not exists
  getItem: (path: string) => string | null | Promise<string | null>;

  setItem: (path: string, value: string) => void | Promise<void>;
  removeItem: (path: string) => void | Promise<void>;
  getAllKeys: (path: string) => string[] | Promise<string[]>;
};

type IVersion = number;

type IMigrate<TState> = (prevState: TState) => Promise<TState> | TState;

export type IPersistoidInputs<TState> = {
  path: string;
  initialState: TState;
  version: IVersion;
  getStorageEngine: () => IStorageEngine | Promise<IStorageEngine>;
  migrations: Record<IVersion, IMigrate<TState> | undefined>;
};

type IPersistoid<TState> = {
  read: () => Promise<TState>;
  write: (state: TState) => Promise<void>;
  getInitialState: () => TState;
};
function createPersistoid<TState>(
  inputs: IPersistoidInputs<TState>
): IPersistoid<TState> {
  let storageEngine: IStorageEngine;

  const setStorageEngine = () =>
    retry(
      'persistoid.setStorageEngine',
      async () => {
        storageEngine = await inputs.getStorageEngine();
      },
      { extraLogData: { tags: { path: inputs.path } } }
    );

  async function read() {
    if (!storageEngine) {
      await setStorageEngine();
    }

    const content = await retry(
      'persistoid.read.getItem',
      async () => {
        const stringifiedContent = await storageEngine.getItem(inputs.path);
        if (stringifiedContent === null) {
          // File does not exist so user didn't have any state persisted, so use
          // initial state.
          return {
            version: inputs.version,
            state: inputs.initialState
          };
        } else {
          return JSON.parse(stringifiedContent);
        }
      },
      { extraLogData: { tags: { path: inputs.path } } }
    );

    const shouldMigrate = content.version !== inputs.version;
    if (shouldMigrate) {
      const initialVersion = content.version;
      const goalVersion = inputs.version;
      const initialState = JSON.stringify(content.state);

      for (
        let nextVersion = content.version + 1;
        nextVersion <= goalVersion;
        nextVersion++
      ) {
        const migrate = inputs.migrations[nextVersion];

        if (migrate) {
          try {
            content.state = await migrate(content.state);

            // Just keep track of version so I can captureSentry this in case a
            // consequent migrate in this for-loop fails.
            content.version = nextVersion;
          } catch (error) {
            const isErrorInstance = error instanceof Error;

            captureSentry(
              isErrorInstance
                ? error
                : new Error(
                    'Non-Error type error during migration, see extras for error'
                  ),
              'persistoid.read.migrate',
              {
                tags: {
                  initialVersion,
                  goalVersion,
                  nextVersion,
                  prevVersion: content.version
                },
                extras: {
                  initialState,
                  error: isErrorInstance ? error : undefined
                }
              }
            );

            throw error;
          }
        }
      }

      // Do it async so that we dont block the user
      write(content.state);
    }

    return content.state;
  }

  const writeLock = new AsyncLock({
    // All but last task are canceled/discarded. Last task has latest state, and
    // all others have outdated state.
    maxPending: Infinity
  });
  const WRITE_TASK_CANCELED = new Error('WRITE_TASK_CANCELED');

  async function write(state: TState) {
    // When support writing multiple files at once, the key will be the path of
    // the file.
    const lockKey = inputs.path;

    // When this is the only task running in the queue,
    // writeLock.queues[lockKey] will be empty. If nothing running for a key,
    // then the lockKey will not exist in the dict of `writeLock.queues`. I only
    // want to run the latest `write` in the queue, so discard all others but
    // the latest one. As the latest one has the latest state. Otherwise we will
    // waste writes writing outdated states.
    const hasPendingTasks = () =>
      lockKey in writeLock.queues && writeLock.queues[lockKey].length > 0;

    const exitIfCanceled = () => {
      if (hasPendingTasks()) {
        addDebugBreadcrumb(
          'persistoid',
          'Write task canceled because a more recent task (and thus with latest state) is pending',
          { queueLength: writeLock.queues[lockKey].length }
        );

        throw WRITE_TASK_CANCELED;
      }
    };

    return writeLock.acquire(lockKey, async function lockedWrite() {
      if (!storageEngine) {
        await setStorageEngine();
      }

      const content = {
        state,
        version: inputs.version
      };

      await retry(
        'persistoid.write.setItem',
        () => {
          exitIfCanceled();

          return storageEngine.setItem(inputs.path, JSON.stringify(content));
        },
        {
          extraLogData: { tags: { path: inputs.path } },
          isTerminalError: error => error === WRITE_TASK_CANCELED
        }
      );
    });
  }

  return { read, write, getInitialState: () => inputs.initialState };
}

export default createPersistoid;

I then connected it to proxy like this:

import { proxy } from 'valtio';
import { subscribeKey } from 'valtio/utils';

import createPersistoid, { IPersistoidInputs } from 'lib/persistoid';

type IProxyWithPersistInputs<T> = IPersistoidInputs<T> & {
  onChange: (data: T) => void;
};

type IBaseState<T> = {
  load: (options?: { shouldLoadInitialState?: boolean }) => Promise<void>;
  loadInitialState: () => Promise<void>;
  write: () => void;
};

type IIdleState<T> = IBaseState<T> & {
  status: 'idle';
  error: null;
  data: null;
};

type ILoadingState<T> = IBaseState<T> & {
  status: 'loading';
  error: null;
  data: null;
};

type ISuccessState<T> = IBaseState<T> & {
  status: 'success';
  error: null;
  data: T;
};

type IFailureState<T> = IBaseState<T> & {
  status: 'error';
  error: any;
  data: null;
};

type IState<T> =
  | IIdleState<T>
  | ILoadingState<T>
  | ISuccessState<T>
  | IFailureState<T>;

export type TProxyWithPersist = ReturnType<typeof proxyWithPersist>;

export type LoadedProxy<T extends TProxyWithPersist> = Extract<
  T,
  { status: 'success' }
>;

function proxyWithPersist<T>(inputs: IProxyWithPersistInputs<T>) {
  const persistoid = createPersistoid<T>(inputs);

  const stateProxy: IState<T> = proxy<IState<T>>({
    status: 'idle',
    error: null,
    data: null,

    // This will never re-load. If already succesfully loaded, it will no-op. If
    // it's loading it will wait for it to finish loading.
    load: async function load(options): Promise<void> {
      if (stateProxy.status === 'success') {
        return;
      }

      if (stateProxy.status === 'loading') {
        return new Promise<void>(resolve => {
          const unsubscribe = subscribeKey(stateProxy, 'status', status => {
            if (status !== 'loading') {
              unsubscribe();
              resolve();
            }
          });
        });
      }

      // If it's in error state, move it to idle state.
      stateProxy.status = 'loading';
      stateProxy.error = null;
      try {
        if (options?.shouldLoadInitialState) {
          stateProxy.data = persistoid.getInitialState();
        } else {
          stateProxy.data = await persistoid.read();
        }

        subscribeKey(stateProxy, 'data', data =>
          inputs.onChange(
            // For sure `data` is now T so it's safe to cast.
            data as T
          )
        );

        stateProxy.status = 'success';
      } catch (error) {
        // Don't log error to Sentry here as persistoid.read() will handle this.
        stateProxy.error = error;
        stateProxy.status = 'error';
      }
    },

    loadInitialState: function loadWithInitialState() {
      return stateProxy.load({ shouldLoadInitialState: true });
    },

    write: function write() {
      if (stateProxy.status === 'success') {
        // sateProxy.success is only set to true once the data has been loaded,
        // so it's safe to cast to T.
        persistoid.write(stateProxy.data as T);
      } else {
        throw new Error('Not loaded, nothing to write');
      }
    }
  });

  return stateProxy;
}

export function tillProxyLoaded(proxy: TProxyWithPersist): Promise<void> {
  if (proxy.status === 'success') {
    return Promise.resolve();
  }

  return new Promise(resolve => {
    const unsubscribe = subscribeKey(proxy, 'status', status => {
      if (
        // @ts-ignore: TS doesn't realize that status can change to 'success'
        status === 'success'
      ) {
        unsubscribe();
        resolve();
      }
    });
  });
}

export default proxyWithPersist;

I then use it like this:

export const myEnrollmentsProxy = proxyWithPersist<TMyEnrollments>({
  path: 'state/my-enrollments.json',
  initialState: [],
  version: 1,
  migrations: {},
  getStorageEngine: () => expoFileEngine,
  onChange: writeMyEnrollments
});

// Use it like this:
myEnrollmentsProxy.data.push({})
dawsonbooth commented 1 year ago

Wow, this is awesome! I'll look into wiring this into what we have - thank you so much!

Noitidart commented 1 year ago

Thanks! It's been working super well. The main blocker for me to get to public is how I did retries and error capturing, not sure if we should ship that by default, but it's been pretty cool, I used debounce as when changes were high, it was wasting resources by writing too much. I'm not sure the API for how to debounce makes sense:

import { debounce } from 'lodash';

const writeMyEnrollments = debounce(function writeMyEnrollments() {
  myEnrollmentsProxy.write();
}, 500);

export type TLoadedMyEnrollmentsProxy = LoadedProxy<typeof myEnrollmentsProxy>;

export const myEnrollmentsProxy = proxyWithPersist<TMyEnrollments>({
  path: 'state/my-enrollments.json',
  initialState: [],
  version: 1,
  migrations: {},
  getStorageEngine: () => expoFileEngine,
  onChange: writeMyEnrollments
});
ScreamZ commented 1 year ago

A simple implementation if you don't want to handle migration, etc… on the web with local storage.

function proxyWithLocalStorage<T extends object>(key: string, initialValue: T) {
  if (typeof window === "undefined") return proxy(initialValue);
  const storageItem = localStorage.getItem(key);

  const state = proxy(storageItem !== null ? (JSON.parse(storageItem) as T) : initialValue);

  subscribe(state, () => localStorage.setItem(key, JSON.stringify(state)));

  return state;
}