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.

Sign up for GitHub

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

feat(world): add namespaceLabel to system config #3057

Merged
merged 11 commits into from
Aug 23, 2024
Merged

feat(world): add namespaceLabel to system config #3057

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
11a3d2d
add namespaceLabel to system, clarify things with comments
holic Aug 22, 2024
b9415f9
validate table namespaceLabel
holic Aug 22, 2024
8e319b3
add system namespace label
holic Aug 22, 2024
d5d63fc
fixes, thanks tests
holic Aug 22, 2024
65ecfb1
more tests
holic Aug 22, 2024
543dbb0
add to cli
holic Aug 22, 2024
6de0cef
Create tasty-toys-deliver.md
holic Aug 22, 2024
1e96f01
Create quick-frogs-fold.md
holic Aug 22, 2024
209521e
copypasta
holic Aug 22, 2024
4e6e0f7
update snapshot
holic Aug 22, 2024
9e46c16
Merge remote-tracking branch 'origin/main' into holic/system-namespac…
holic Aug 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/quick-frogs-fold.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@latticexyz/config": patch
"@latticexyz/store": patch
---

Fixed a few type issues with `namespaceLabel` in tables and added/clarified TSDoc for config input/output objects.
7 changes: 7 additions & 0 deletions .changeset/tasty-toys-deliver.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@latticexyz/cli": patch
"@latticexyz/world": patch
---

Add a strongly typed `namespaceLabel` to the system config output.
It corresponds to the `label` of the namespace the system belongs to and can't be set manually.
7 changes: 6 additions & 1 deletion packages/cli/src/deploy/common.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,19 +82,24 @@ export type Library = DeterministicContract & {
};

export type System = DeterministicContract & {
// labels
readonly label: string;
readonly namespaceLabel: string;
// resource ID
readonly namespace: string;
readonly name: string;
readonly systemId: Hex;
// access
readonly allowAll: boolean;
readonly allowedAddresses: readonly Hex[];
readonly allowedSystemIds: readonly Hex[];
// world registration
readonly worldFunctions: readonly WorldFunction[];
};

export type DeployedSystem = Omit<
System,
"label" | "abi" | "prepareDeploy" | "deployedBytecodeSize" | "allowedSystemIds"
"label" | "namespaceLabel" | "abi" | "prepareDeploy" | "deployedBytecodeSize" | "allowedSystemIds"
> & {
address: Address;
};
Expand Down
28 changes: 27 additions & 1 deletion packages/config/src/common.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,39 @@ export type Schema = {
};

export type Table = {
/**
* Human-readable label for this table. Used as config keys, library names, and filenames.
* Labels are not length constrained like resource names, but special characters should be avoided to be compatible with the filesystem, Solidity compiler, etc.
Copy link
Contributor

@karooolis karooolis Aug 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wonder if these special characters should be specified here? I realize "special character" has a pretty universal meaning but it can still be unclear if "-" is allowed, for example. Having said that, curious if there are any other constraints like can a label start with number, etc?

Copy link
Member Author

@holic holic Aug 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know the range of special characters in various OS's so I'll leave it to user discretion

if a label has a non-fs compatible character, they'll know quickly because tablegen/worldgen will fail to write files named with those labels

*/
readonly label: string;
/**
* Human-readable label for this table's namespace. Used for namespace config keys and directory names.
*/
readonly namespaceLabel: string;
/**
* Table type used in table's resource ID and determines how storage and events are used by this table.
*/
readonly type: satisfy<ResourceType, "table" | "offchainTable">;
/**
* Table namespace used in table's resource ID and determines access control.
*/
readonly namespace: string;
readonly namespaceLabel: string;
/**
* Table name used in table's resource ID.
*/
readonly name: string;
/**
* Table's resource ID.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like the property descriptions. Just curious is there a reason why it uses single-line comments in packages/cli/src/deploy/common.ts but then multi-line comments here, even for single line comments?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that file is internal, so not as important to have good docs and those are mostly internal notes

the tsdoc here will get included as part of your IDE tooltips and maybe eventually generated into documentation

*/
readonly tableId: Hex;
/**
* Schema definition for this table's records.
*/
readonly schema: Schema;
/**
* Primary key for records of this table. An array of zero or more schema field names.
* Using an empty array acts like a singleton, where only one record can exist for this table.
*/
readonly key: readonly string[];
};

Expand Down
4 changes: 2 additions & 2 deletions packages/store/ts/config/v2/defaults.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ export const TABLE_DEPLOY_DEFAULTS = {
export type TABLE_DEPLOY_DEFAULTS = typeof TABLE_DEPLOY_DEFAULTS;

export const TABLE_DEFAULTS = {
namespace: "",
namespaceLabel: "",
type: "table",
Comment on lines +27 to 28
Copy link
Contributor

@karooolis karooolis Aug 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just wonder why only these two properties are used as TABLE_DEFAULTS? Why is namespace not part of, and all the other properties in TableInput type?

Copy link
Contributor

@karooolis karooolis Aug 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see now, namespace defaults to namespaceLabel in resolveSystem.

} as const satisfies Pick<TableInput, "namespace" | "type">;
} as const satisfies Pick<TableInput, "namespaceLabel" | "type">;

export type TABLE_DEFAULTS = typeof TABLE_DEFAULTS;

Expand Down
22 changes: 15 additions & 7 deletions packages/store/ts/config/v2/input.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,17 @@ export type TableDeployInput = Partial<TableDeploy>;

export type TableInput = {
/**
* Human-readable table label. Used as config keys, table library names, and filenames.
* Human-readable label for this table. Used as config keys, library names, and filenames.
* Labels are not length constrained like resource names, but special characters should be avoided to be compatible with the filesystem, Solidity compiler, etc.
*/
readonly label: string;
/**
* Human-readable label for this table's namespace. Used for namespace config keys and directory names.
* Defaults to the nearest namespace in the config or root namespace if not set.
*/
readonly namespaceLabel?: string;
Copy link
Contributor

@karooolis karooolis Aug 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably too big of a change now and not worth the effort but wonder if defining namespace as its own type would make sense e.g:

type NamespaceInput = {
  id: string;
  label?: string;
}

type Table Input = {
  namespace?: NamespaceInput;
  // ...
}

Copy link
Member Author

@holic holic Aug 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this would be a breaking change unfortunately

namespaceLabel here won't actually be used by users, it's inferred/propagated from outer context (tables defined inside a namespace in the store/world config)

/**
* Table type used in table's resource ID and determines how storage and events are used by this table.
* Defaults to `table` if not set.
*/
readonly type?: "table" | "offchainTable";
Expand All @@ -31,17 +37,19 @@ export type TableInput = {
* Defaults to the nearest namespace in the config or root namespace if not set.
*/
readonly namespace?: string;
/**
* Human-readable namespace label.
* Defaults to the nearest namespace in the config or root namespace if not set.
*/
readonly namespaceLabel?: string;
/**
* Table name used in table's resource ID.
* Defaults to the first 16 characters of `label` if not set.
*/
readonly name?: string;
/**
* Schema definition for this table's records.
*/
readonly schema: SchemaInput;
/**
* Primary key for records of this table. An array of zero or more schema field names.
* Using an empty array acts like a singleton, where only one record can exist for this table.
*/
readonly key: readonly string[];
readonly codegen?: TableCodegenInput;
readonly deploy?: TableDeployInput;
Expand All @@ -52,7 +60,7 @@ export type TableShorthandInput = SchemaInput | string;
export type TablesInput = {
// remove label and namespace as these are set contextually
// and allow defining a table using shorthand
readonly [label: string]: Omit<TableInput, "label" | "namespace" | "namespaceLabel"> | TableShorthandInput;
readonly [label: string]: Omit<TableInput, "label" | "namespaceLabel" | "namespace"> | TableShorthandInput;
};

export type CodegenInput = Partial<Codegen>;
Expand Down
67 changes: 55 additions & 12 deletions packages/store/ts/config/v2/store.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ describe("defineStore", () => {
readonly tables: {
readonly Example: {
readonly label: "Example"
readonly type: "table"
readonly namespaceLabel: ""
readonly type: "table"
readonly namespace: string
readonly name: string
readonly tableId: \`0x\${string}\`
Expand Down Expand Up @@ -115,8 +115,8 @@ describe("defineStore", () => {
readonly tables: {
readonly Example: {
readonly label: "Example"
readonly type: "table"
readonly namespaceLabel: ""
readonly type: "table"
readonly namespace: string
readonly name: string
readonly tableId: \`0x\${string}\`
Expand Down Expand Up @@ -237,8 +237,8 @@ describe("defineStore", () => {
readonly tables: {
readonly Example: {
readonly label: "Example"
readonly type: "table"
readonly namespaceLabel: "root"
readonly type: "table"
readonly namespace: string
readonly name: string
readonly tableId: \`0x\${string}\`
Expand Down Expand Up @@ -271,8 +271,8 @@ describe("defineStore", () => {
readonly tables: {
readonly root__Example: {
readonly label: "Example"
readonly type: "table"
readonly namespaceLabel: "root"
readonly type: "table"
readonly namespace: string
readonly name: string
readonly tableId: \`0x\${string}\`
Expand Down Expand Up @@ -651,36 +651,56 @@ describe("defineStore", () => {
namespace: "CustomNS",
tables: {
Example: {
// @ts-expect-error "Overrides of `label` and `namespace` are not allowed for tables in this context"
// @ts-expect-error "Overrides of `label`, `namespaceLabel`, and `namespace` are not allowed for tables in this context"
label: "NotAllowed",
schema: { id: "address" },
key: ["id"],
},
},
}),
).throwsAndHasTypeError("Overrides of `label` and `namespace` are not allowed for tables in this context");
).throwsAndHasTypeError(
"Overrides of `label`, `namespaceLabel`, and `namespace` are not allowed for tables in this context",
);

attest(() =>
defineStore({
namespace: "CustomNS",
tables: {
Example: {
// @ts-expect-error "Overrides of `label` and `namespace` are not allowed for tables in this context"
// @ts-expect-error "Overrides of `label`, `namespaceLabel`, and `namespace` are not allowed for tables in this context"
namespaceLabel: "NotAllowed",
schema: { id: "address" },
key: ["id"],
},
},
}),
).throwsAndHasTypeError(
"Overrides of `label`, `namespaceLabel`, and `namespace` are not allowed for tables in this context",
);

attest(() =>
defineStore({
namespace: "CustomNS",
tables: {
Example: {
// @ts-expect-error "Overrides of `label`, `namespaceLabel`, and `namespace` are not allowed for tables in this context"
namespace: "NotAllowed",
schema: { id: "address" },
key: ["id"],
},
},
}),
).throwsAndHasTypeError("Overrides of `label` and `namespace` are not allowed for tables in this context");
).throwsAndHasTypeError(
"Overrides of `label`, `namespaceLabel`, and `namespace` are not allowed for tables in this context",
);

attest(() =>
defineStore({
namespaces: {
CustomNS: {
tables: {
Example: {
// @ts-expect-error "Overrides of `label` and `namespace` are not allowed for tables in this context"
// @ts-expect-error "Overrides of `label`, `namespaceLabel`, and `namespace` are not allowed for tables in this context"
label: "NotAllowed",
schema: { id: "address" },
key: ["id"],
Expand All @@ -689,15 +709,36 @@ describe("defineStore", () => {
},
},
}),
).throwsAndHasTypeError("Overrides of `label` and `namespace` are not allowed for tables in this context");
).throwsAndHasTypeError(
"Overrides of `label`, `namespaceLabel`, and `namespace` are not allowed for tables in this context",
);

attest(() =>
defineStore({
namespaces: {
CustomNS: {
tables: {
Example: {
// @ts-expect-error "Overrides of `label` and `namespace` are not allowed for tables in this context"
// @ts-expect-error "Overrides of `label`, `namespaceLabel`, and `namespace` are not allowed for tables in this context"
namespaceLabel: "NotAllowed",
schema: { id: "address" },
key: ["id"],
},
},
},
},
}),
).throwsAndHasTypeError(
"Overrides of `label`, `namespaceLabel`, and `namespace` are not allowed for tables in this context",
);

attest(() =>
defineStore({
namespaces: {
CustomNS: {
tables: {
Example: {
// @ts-expect-error "Overrides of `label`, `namespaceLabel`, and `namespace` are not allowed for tables in this context"
namespace: "NotAllowed",
schema: { id: "address" },
key: ["id"],
Expand All @@ -706,7 +747,9 @@ describe("defineStore", () => {
},
},
}),
).throwsAndHasTypeError("Overrides of `label` and `namespace` are not allowed for tables in this context");
).throwsAndHasTypeError(
"Overrides of `label`, `namespaceLabel`, and `namespace` are not allowed for tables in this context",
);
});

it("should allow const enum as input", () => {
Expand Down
23 changes: 15 additions & 8 deletions packages/store/ts/config/v2/table.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export type ValidateTableOptions = { inStoreContext: boolean };

export type requiredTableKey<inStoreContext extends boolean> = Exclude<
requiredKeyOf<TableInput>,
inStoreContext extends true ? "label" | "namespace" : ""
inStoreContext extends true ? "label" | "namespaceLabel" | "namespace" : never
>;

export type validateTable<
Expand All @@ -59,9 +59,9 @@ export type validateTable<
? validateKeys<getStaticAbiTypeKeys<conform<get<input, "schema">, SchemaInput>, scope>, get<input, key>>
: key extends "schema"
? validateSchema<get<input, key>, scope>
: key extends "label" | "namespace"
: key extends "label" | "namespaceLabel" | "namespace"
? options["inStoreContext"] extends true
? ErrorMessage<"Overrides of `label` and `namespace` are not allowed for tables in this context">
? ErrorMessage<"Overrides of `label`, `namespaceLabel`, and `namespace` are not allowed for tables in this context">
: key extends keyof input
? narrow<input[key]>
: never
Expand Down Expand Up @@ -115,8 +115,13 @@ export function validateTable<input, scope extends Scope = AbiTypeScope>(
throw new Error(`Table \`name\` must fit into a \`bytes16\`, but "${input.name}" is too long.`);
}

if (options.inStoreContext && (hasOwnKey(input, "label") || hasOwnKey(input, "namespace"))) {
throw new Error("Overrides of `label` and `namespace` are not allowed for tables in this context.");
if (
options.inStoreContext &&
(hasOwnKey(input, "label") || hasOwnKey(input, "namespaceLabel") || hasOwnKey(input, "namespace"))
) {
throw new Error(
"Overrides of `label`, `namespaceLabel`, and `namespace` are not allowed for tables in this context.",
);
}
}

Expand Down Expand Up @@ -151,10 +156,10 @@ export function resolveTableCodegen<input extends TableInput>(input: input): res
export type resolveTable<input, scope extends Scope = Scope> = input extends TableInput
? {
readonly label: input["label"];
readonly type: undefined extends input["type"] ? typeof TABLE_DEFAULTS.type : input["type"];
readonly namespaceLabel: undefined extends input["namespaceLabel"]
? typeof TABLE_DEFAULTS.namespace
? typeof TABLE_DEFAULTS.namespaceLabel
: input["namespaceLabel"];
readonly type: undefined extends input["type"] ? typeof TABLE_DEFAULTS.type : input["type"];
readonly namespace: string;
readonly name: string;
readonly tableId: Hex;
Expand All @@ -171,8 +176,10 @@ export function resolveTable<input extends TableInput, scope extends Scope = Abi
input: input,
scope: scope = AbiTypeScope as unknown as scope,
): resolveTable<input, scope> {
const namespaceLabel = input.namespaceLabel ?? TABLE_DEFAULTS.namespace;
const namespaceLabel = input.namespaceLabel ?? TABLE_DEFAULTS.namespaceLabel;
// validate ensures this is length constrained
const namespace = input.namespace ?? namespaceLabel;
Copy link
Contributor

@karooolis karooolis Aug 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find it slightly odd that namespace is assigned with namespaceLabel in case it's undefined. This means that namespaceLabel is treated as the default ID. I'd expect it to be in reverse but I suspect it's due to some constraints I'm not aware of yet.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

namespaceLabel is the "entry point" and the strongly typed thing, which propagates to the namespace if it meets namespace criteria (<=14 chars) of the resource ID, otherwise throws in the validate function and requires either adjusting the label or specifying an explicit namespace


const label = input.label;
const name = input.name ?? label.slice(0, 16);
const type = input.type ?? TABLE_DEFAULTS.type;
Expand Down
4 changes: 3 additions & 1 deletion packages/store/vitestSetup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
export { setup, cleanup as teardown } from "@ark/attest";
import { setup } from "@ark/attest";

export default () => setup({ updateSnapshots: true });
4 changes: 2 additions & 2 deletions packages/world/ts/config/v2/defaults.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ export const SYSTEM_DEPLOY_DEFAULTS = {
export type SYSTEM_DEPLOY_DEFAULTS = typeof SYSTEM_DEPLOY_DEFAULTS;

export const SYSTEM_DEFAULTS = {
namespace: "",
namespaceLabel: "",
openAccess: true,
accessList: [],
} as const satisfies Omit<Required<SystemInput>, "label" | "name" | "deploy">;
} as const satisfies Omit<Required<SystemInput>, "label" | "namespace" | "name" | "deploy">;

export type SYSTEM_DEFAULTS = typeof SYSTEM_DEFAULTS;

Expand Down
7 changes: 6 additions & 1 deletion packages/world/ts/config/v2/input.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ export type SystemInput = {
* Labels are not length constrained like resource names, but special characters should be avoided to be compatible with the filesystem, Solidity compiler, etc.
*/
readonly label: string;
/**
* Human-readable label for this system's namespace. Used for namespace config keys and directory names.
* Defaults to the nearest namespace in the config or root namespace if not set.
*/
readonly namespaceLabel?: string;
/**
* System namespace used in systems's resource ID and determines access control.
* Defaults to the nearest namespace in the config or root namespace if not set.
Expand All @@ -28,7 +33,7 @@ export type SystemInput = {
};

export type SystemsInput = {
readonly [label: string]: Omit<SystemInput, "label" | "namespace">;
readonly [label: string]: Omit<SystemInput, "label" | "namespaceLabel" | "namespace">;
};

export type NamespaceInput = StoreNamespaceInput & {
Expand Down
Loading
Loading