feat(SyncExternalStore): Add StatefulSyncExternalStore to extend and provide structure

[mmcxii]

No description provided.

[k2snowman69]

This doesn't seem right... I don't think your typescript types are flowing through correctly. I need to pull this down...

[k2snowman69]

Can you create unit tests to help document usage? Also you know, code coverage?

[k2snowman69]

I pulled this down... in the original context-store there were three store types(copied from the readme):

• useContextStore - For non-indexable stores like objects and primatives or when you don't need to edit individual elements
• useIndexableContextStore - For indexable stores like maps or arrays with a single loading state at the root
• useIndexableStatefulContextStore - For indexable stores but when you want to maintain separate load states per item than the list of items

From the StoreState object you created, this looks to be specifically IndexableStateful

It seems it might make sense to have three classes then:

• StatefulSyncExternalStore
• IndexableStatefulSyncExternalStore
• IndexableStatefulItemSyncExternalStore

Or something...

[k2snowman69]

Nit, rename this to action as it's not inherently tied to a service call. Also to make this more flexible consider a return type of (params: TParams) => Promise<TResponse> | TResponse; so it can handle sync and async even though the sync scenario doesn't make a ton of sense

[k2snowman69]

Okay playing with this the code I got to was something like this:

  public getOne(id: string) {
    return this.updateStore({
      key: id,
      service: (id: string) => Promise.resolve("hello" + id),
    });
  }

which is a bit weird that id is reused soo many times. Also the only thing that changes here after initialization is id and params, service and updateSnapshot should be constants through the lifetime of the class.

I think there's some code reorganization needed to split these apart

[k2snowman69]

Maybe this is actually a good use of currying weirdly...
Edit: Nope, nevermind I think I'm wrong here..

[k2snowman69]

Here's the dumb non-currying version of some sample code which would be nice to be able to write (if we can get it be less lines of code anyway)

class SomeStore extends StatefulSyncExternalStore<Record<string, StoreItem<string>>> {
  private getOneService = this.createUpdateStore({
    service: (id: string) => Promise.resolve("hello" + id),
  });
  public getOne(id: string) {
    return this.getOneService({
      key: "id",
      params: id,
    });
  }
}

function run() {
  const d = new SomeStore({});
  d.getOne("id-#1");
}

[k2snowman69]

This as TParams screams either bard code implementation or bad typing... I think my suggestion to split up the function into a factory createUpdateStore will help you fix this type

[k2snowman69]

I think the biggest changes required here is:

1. Check out the difference between useContextStore, useIndexibleContextStore, and useStatefulIndexibleContextStore. You've implemented one of the three here.
2. UpdateStoreParams can be split into two parts, the first is creation (service, updateSnapshot) and the second is action (key, params). If you split this, I think your types and readability get better.
3. Add a simple unit test so it's easy to see how you expect to use this in code

Once we iterate on that the last steps are:

1. Add unit tests to get coverage as high as possible
2. Add JSDoc if needed
3. Add instructions in readme

[k2snowman69]

Leaving a note to update the CHANGELOG and README after we lock down the code

[k2snowman69]

Mmm not quite... you should snapshot.data should be an array (which is an object). You should be able to just do snapshot.data[data.id] = data to set the data. This is a map lookup. Indexible means you are looking things based on a particular index-able value.

[k2snowman69]

This one looks good to me... effectively do the same with the sync-external-store--indexable but move the state up a level and you're done... and also handle concurrent requests in your example code

[k2snowman69]

This one looks good to me too, again concurrency is a good example to have somewhere in your test