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

stash improvement ideas #3333

Open
holic opened this issue Oct 25, 2024 · 5 comments
Open

stash improvement ideas #3333

holic opened this issue Oct 25, 2024 · 5 comments

Comments

@holic
Copy link
Member

holic commented Oct 25, 2024

  • collapse getRecord and getRecords into a single get function that has a few optional parameters (e.g. key or keys)

    // get records
    useStash(stash, (state) => get({ state, table }))
    useStash(stash, (state) => get({ state, table, keys: [key1, key2] }))
    // get record
    useStash(stash, (state) => get({ state, table, key }))

    could even collapse further if get returned a selector like

    // get records
    useStash(stash, get({ table }))
    useStash(stash, get({ table, keys: [key1, key2] }))
    // get record
    useStash(stash, get({ table, key }))
  • as a general rule, provide helpers to simplify usage/ergonomics that lead to data patterns we can optimize well (see below about shallow equality checks), but still allow for raw selectors for folks that want more advanced use and are willing to do the optimization themselves

  • consider defaulting to shallow equality check to avoid passing in an equality fn when getting records by keys, because this always returns a new object and won't pass the strict equality (===) check

    • warn on big shallow checks i.e. objects with many keys or arrays with many values)
  • add stash context/provider for setting global defaults

    • default stash to enable hook usage like useStash((state) => ...) or with above get selector like useStash(get(...))
    • default equality fn if you want to always use a deep equal or similar
@holic
Copy link
Member Author

holic commented Oct 25, 2024

A big hurdle for using Stash as a plain client data store is the strict data types using EVM-based schemas, which limits storing things like JS native objects, using string keys, etc.

We could lift out the performant bits of Stash (a strongly typed mutable object tree that uses tables with keys/records as its primary constraint) and then build the MUD Store-flavored version on top that turns Store config/tables into a more restricted set of data + helpers for working with it (i.e. encoding table names, keys).

type Table = {
  readonly [fieldName: string]: any;
};

type Tables = {
  readonly [tableName: string]: Table;
}

type State<tables extends Tables> = {
  readonly tables: tables;
  readonly records: {
    readonly [tableName in keyof tables]: {
      readonly [key: string]: TableRecord<tables[tableName]>;
    };
  };
};

type MutableState<tables extends Tables> = {
  // same as above, but mutable
};

type Stash<tables extends Tables> = {
  state: MutableState<tables>
};

Note that this would mean flattening the current Stash internal representation from records[namespace][tableName] to something like records[`${namespace}__${table}`] but I think that's a fine/worthy trade-off (I don't think we have any APIs that return namespace-specific data anyway).

@holic
Copy link
Member Author

holic commented Oct 28, 2024

Thinking more on the get idea above, it might be better to just have separate hooks that we can individually optimize:

useRecord({ stash, table, key })
useRecords({ stash, table })
useRecords({ stash, table, keys })

@alvrs
Copy link
Member

alvrs commented Oct 28, 2024

Some observations from my side:

  • we should rethink the update type of subscribeQuery, it's currently really hard to work with
  • we should align the type of subscribeTable and subscribeQuery (currently one takes a subscriber as argument, the other returns an object with a subscribe function)

@holic
Copy link
Member Author

holic commented Oct 28, 2024

Just realized that the current useStash implementation that leans on React's useSyncExternalStore (for good perf+minimal re-renders+alignment with zustand) means we don't get the list of updates you'd normally get from a subscription.

Might be nice to have an alternative hook to opt into updates if you need em.

@alvrs
Copy link
Member

alvrs commented Oct 29, 2024

CleanShot 2024-10-29 at 00 41 21
CleanShot 2024-10-29 at 00 41 41@2x

Need to investigate whether Matches actually filters by all provided options 🤔

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

No branches or pull requests

3 participants
@holic @alvrs and others