Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Whitelisted properties do not persist for newer valtio versions #7

Open
dawsonbooth opened this issue Oct 18, 2022 · 7 comments
Open

Comments

@dawsonbooth
Copy link
Contributor

dawsonbooth commented Oct 18, 2022

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
Copy link
Owner

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
Copy link
Contributor Author

Sounds great, thanks for the quick response!

@dawsonbooth
Copy link
Contributor Author

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

@Noitidart
Copy link
Owner

Noitidart commented Dec 21, 2022

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
Copy link
Contributor Author

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

@Noitidart
Copy link
Owner

Noitidart commented Dec 23, 2022

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
Copy link

ScreamZ commented Mar 31, 2023

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;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants