feat(store-sync): extra table definitions #1840
Conversation
🦋 Changeset detectedLatest commit: b9e8c6f The changes in this PR will be included in the next version bump. This PR includes changesets to release 30 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| @@ -22,7 +34,7 @@ export type ResolvedTableConfig< | |||
| TEnumNames extends StringForUnion, | |||
| TNamespace extends string = string, | |||
| TName extends string = string | |||
| > = Omit<TTableConfig, "keySchema" | "valueSchema"> & { | |||
There was a problem hiding this comment.
I didn't want to clutter the new resolved config/tables shape with old props that we're gonna rearrange, so I removed em. Can still access em via the old config object and the same table name/key.
| @@ -48,7 +60,9 @@ export type ResolvedSchema< | |||
| TEnumNames extends StringForUnion | |||
| > = { | |||
| [key in keyof TSchema]: { | |||
| type: TSchema[key] extends keyof TUserTypes | |||
| type: TSchema[key] extends SchemaAbiType | |||
| ? TSchema[key] | |||
There was a problem hiding this comment.
@alvrs This was the fix. Not sure why only certain ABI types were getting trapped in the TSchema[key] extends TEnumNames check.
| @@ -71,7 +71,7 @@ const zShorthandSchemaConfig = zFieldData.transform((fieldData) => { | |||
|
|
|||
| export const zSchemaConfig = zFullSchemaConfig.or(zShorthandSchemaConfig); | |||
|
|
|||
| export type ResolvedSchema< | |||
| type ResolvedSchema< | |||
There was a problem hiding this comment.
not used anywhere and the name overlaps with our experimental config types, so I removed this export
| config: mudConfig, | ||
| tables: { |
There was a problem hiding this comment.
so syncToRecs syncs all tables passed mudConfig or in tables correct? What in mudConfig do we need in here besides the resolved tables? If it's only that, should we just pass a tables key (ie syncToRecs({ tables: { ...mudConfig.tables, ...additionalTables } }))? (mostly wondering about longer term when the config is resolved by default, fine to not do that here yet to avoid breaking changes)
There was a problem hiding this comment.
long term I think we should do that, yep! I just didn't want to encourage the intermediate step of using the experimental resolveConfig in user land
| address: networkConfig.worldAddress as Hex, | ||
| publicClient, | ||
| startBlock: BigInt(networkConfig.initialBlockNumber), | ||
| }); | ||
| } as const); |
There was a problem hiding this comment.
what would break without as const here? Does this break anything for current users who don't have as const yet?
There was a problem hiding this comment.
the manually defined tables were missing strong types for namespace/name without this
| export type RecsStorageAdapter<TConfig extends StoreConfig = StoreConfig> = { | ||
| export type RecsStorageAdapter<tables extends Record<string, Table>> = { |
There was a problem hiding this comment.
I'm still not used to tables instead of TTables 😭 Don't you find it confusing not to see on first sight whether you have to use typeof tables vs just tables? I'm just so used to custom types being uppercase and only primitive types being lowercase
holic
changed the title
before
after