feat(config,store,world): add namespaceLabel to table config #3039
Conversation
🦋 Changeset detectedLatest commit: 7119805 The changes in this PR will be included in the next version bump. This PR includes changesets to release 25 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 |
packages/store/ts/config/v2/table.ts
Outdated
| @@ -159,13 +164,15 @@ export function resolveTable<input extends TableInput, scope extends Scope = Abi | |||
| const label = input.label; | |||
| const type = input.type ?? TABLE_DEFAULTS.type; | |||
| const namespace = input.namespace ?? TABLE_DEFAULTS.namespace; | |||
| const namespaceLabel = input.namespaceLabel ?? input.namespace ?? TABLE_DEFAULTS.namespace; | |||
There was a problem hiding this comment.
| const namespaceLabel = input.namespaceLabel ?? input.namespace ?? TABLE_DEFAULTS.namespace; | |
| const namespaceLabel = input.namespaceLabel ?? namespace; |
for names, this is actually the other way around: user defines the name or it defaults to the label
I wonder if we should do the same here: allow specifying namespace label, default it if not set, then derive the actual namespace from either the input or the namespace label (and validate length?)
There was a problem hiding this comment.
good point, switched the logic around
| const label = input.label; | ||
| const namespace = input.namespace ?? label.slice(0, 14); | ||
| const namespaceLabel = input.label; | ||
| const namespace = input.namespace ?? namespaceLabel.slice(0, 14); |
There was a problem hiding this comment.
I think we may wanna throw here instead of truncating?
I ran into this at some point where it's unclear if its okay to throw in resolve step, like as if it should happen in the validate step. But maybe its fine because its a "resolve error" because we're filling in and inferring info? Or maybe in this case, we can validate the namespaceLabel is <= 14 chars only if namespace isn't supplied too.
There was a problem hiding this comment.
added the validation logic including throwing to validateTable. Could throw here for sanity but we should be able to assume the input to resolveX is sufficiently validated
There was a problem hiding this comment.
Ah hadn't pushed yet. Opted for not throwing here for now and putting all validation in validateTable (we weren't throwing if namespace is too long here either)
Ripped out from #2944