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

refactor(store): update tablegen with namespaces output #2972

Merged
merged 16 commits into from
Jul 26, 2024
8 changes: 8 additions & 0 deletions .changeset/shaggy-weeks-grab.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
"@latticexyz/store": patch
---

Refactored tablegen in preparation for multiple namespaces and addressed a few edge cases:

- User types configured with a relative `filePath` are now resolved relative to the project root (where the `mud.config.ts` lives) rather than the current working directory.
- User types inside libraries now need to be referenced with their fully-qualified code path (e.g. `LibraryName.UserTypeName`).

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/cli/contracts/src/codegen/tables/Statics.sol

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/cli/contracts/src/codegen/tables/UserTyped.sol

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

21 changes: 9 additions & 12 deletions packages/cli/scripts/generate-test-tables.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { tablegen } from "@latticexyz/store/codegen";
import { defineStore } from "@latticexyz/store";
import { getRemappings } from "@latticexyz/common/foundry";
import { fileURLToPath } from "node:url";
import path from "node:path";

Expand All @@ -16,10 +15,10 @@ const config = defineStore({
Enum2: ["E1"],
},
userTypes: {
TestTypeAddress: { filePath: "./contracts/src/types.sol", type: "address" },
TestTypeInt64: { filePath: "./contracts/src/types.sol", type: "int64" },
TestTypeBool: { filePath: "./contracts/src/types.sol", type: "bool" },
TestTypeUint128: { filePath: "./contracts/src/types.sol", type: "uint128" },
TestTypeAddress: { filePath: "../contracts/src/types.sol", type: "address" },
Copy link
Member Author

@holic holic Jul 25, 2024

Choose a reason for hiding this comment

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

previously, codegen expected this path to be relative to process.cwd() (almost always the project root/same dir as the mud.config.ts)

once I changed the paths to be relative to the project root/mud.config.ts, this broke, because for this particular script, the config is in a slightly different place

this was an inconsistency/edge case that should now be more consistent and predictable and enables better resolving of user types from within namespaced directories

TestTypeInt64: { filePath: "../contracts/src/types.sol", type: "int64" },
"TestTypeLibrary.TestTypeBool": { filePath: "../contracts/src/types.sol", type: "bool" },
"TestTypeLibrary.TestTypeUint128": { filePath: "../contracts/src/types.sol", type: "uint128" },
Copy link
Member Author

Choose a reason for hiding this comment

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

previously, codegen would parse the file at filePath to determine where in the file was the specified user type and then propagate the fully qualified name everywhere

this felt too "automagic" to me and added a lot of complexity to codegen that I was able to remove by not automatically inferring the code path for the user type (with the nice side effect that we no longer need remappings for tablegen)

we can still support this (imo rare) case by specifying the fully qualified type name like above

(maybe in the future could improve this with a similar "label" approach where the label is used in config references like schemas, and allow specifying a fully qualified code path override of internalType that gets used in codegen)

ResourceId: { filePath: "@latticexyz/store/src/ResourceId.sol", type: "bytes32" },
},
tables: {
Expand Down Expand Up @@ -82,20 +81,18 @@ const config = defineStore({
schema: {
k1: "TestTypeAddress",
k2: "TestTypeInt64",
k3: "TestTypeBool",
k4: "TestTypeUint128",
k3: "TestTypeLibrary.TestTypeBool",
k4: "TestTypeLibrary.TestTypeUint128",
k5: "ResourceId",
v1: "TestTypeAddress",
v2: "TestTypeInt64",
v3: "TestTypeBool",
v4: "TestTypeUint128",
v3: "TestTypeLibrary.TestTypeBool",
v4: "TestTypeLibrary.TestTypeUint128",
v5: "ResourceId",
},
key: ["k1", "k2", "k3", "k4", "k5"],
},
},
});

const remappings = await getRemappings();

await tablegen({ rootDir: path.dirname(configPath), config, remappings });
await tablegen({ rootDir: path.dirname(configPath), config });
6 changes: 2 additions & 4 deletions packages/cli/src/build.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { tablegen } from "@latticexyz/store/codegen";
import { worldgen } from "@latticexyz/world/node";
import { World as WorldConfig } from "@latticexyz/world";
import { forge, getRemappings } from "@latticexyz/common/foundry";
import { forge } from "@latticexyz/common/foundry";
import { execa } from "execa";

type BuildOptions = {
Expand All @@ -20,9 +20,7 @@ export async function build({
config,
foundryProfile = process.env.FOUNDRY_PROFILE,
}: BuildOptions): Promise<void> {
const remappings = await getRemappings(foundryProfile);

await Promise.all([tablegen({ rootDir, config, remappings }), worldgen({ rootDir, config })]);
await Promise.all([tablegen({ rootDir, config }), worldgen({ rootDir, config })]);

await forge(["build"], { profile: foundryProfile });
await execa("mud", ["abi-ts"], { stdio: "inherit" });
Expand Down
4 changes: 1 addition & 3 deletions packages/cli/src/commands/tablegen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import type { CommandModule } from "yargs";
import { loadConfig, resolveConfigPath } from "@latticexyz/config/node";
import { Store as StoreConfig } from "@latticexyz/store";
import { tablegen } from "@latticexyz/store/codegen";
import { getRemappings } from "@latticexyz/common/foundry";
import path from "node:path";

type Options = {
Expand All @@ -23,9 +22,8 @@ const commandModule: CommandModule<Options, Options> = {
async handler(opts) {
const configPath = await resolveConfigPath(opts.configPath);
const config = (await loadConfig(configPath)) as StoreConfig;
const remappings = await getRemappings();

await tablegen({ rootDir: path.dirname(configPath), config, remappings });
await tablegen({ rootDir: path.dirname(configPath), config });

process.exit(0);
},
Expand Down
8 changes: 6 additions & 2 deletions packages/common/src/codegen/render-solidity/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
import { posixPath } from "../utils";
import { resourceToHex } from "../../resourceToHex";
import { hexToResource } from "../../hexToResource";
import { renderImportPath } from "./renderImportPath";

/**
* Common header for all codegenerated solidity files
Expand Down Expand Up @@ -72,7 +73,10 @@ export function renderCommonData({
};
}

/** For 2 paths which are relative to a common root, create a relative import path from one to another */
/**
* For 2 paths which are relative to a common root, create a relative import path from one to another
* @deprecated Use `renderImportPath` instead.
*/
export function solidityRelativeImportPath(fromPath: string, usedInPath: string): string {
// 1st "./" must be added because path strips it,
// but solidity expects it unless there's "../" ("./../" is fine).
Expand Down Expand Up @@ -129,7 +133,7 @@ export function renderAbsoluteImports(imports: AbsoluteImportDatum[]): string {
const renderedImports = [];
for (const [path, symbols] of aggregatedImports) {
const renderedSymbols = [...symbols].join(", ");
renderedImports.push(`import { ${renderedSymbols} } from "${posixPath(path)}";`);
renderedImports.push(`import { ${renderedSymbols} } from "${renderImportPath(path)}";`);
}
return renderedImports.join("\n");
}
Expand Down
1 change: 1 addition & 0 deletions packages/common/src/codegen/render-solidity/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export * from "./common";
export * from "./renderEnums";
export * from "./renderImportPath";
export * from "./renderTypeHelpers";
export * from "./types";
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
import { describe, expect, it } from "vitest";
import { renderImportPath } from "./renderImportPath";

describe("renderImportPath", () => {
it("returns valid path for package imports", () => {
expect(renderImportPath("@latticexyz/store/src")).toMatchInlineSnapshot('"@latticexyz/store/src"');
expect(renderImportPath("@latticexyz/store/src/")).toMatchInlineSnapshot('"@latticexyz/store/src"');
expect(renderImportPath("@latticexyz/store/src", "IStore.sol")).toMatchInlineSnapshot(
'"@latticexyz/store/src/IStore.sol"',
);
expect(renderImportPath("@latticexyz/store/src/", "IStore.sol")).toMatchInlineSnapshot(
'"@latticexyz/store/src/IStore.sol"',
);
expect(renderImportPath("@latticexyz/store/src", "codegen/tables/Tables.sol")).toMatchInlineSnapshot(
'"@latticexyz/store/src/codegen/tables/Tables.sol"',
);
expect(renderImportPath("@latticexyz/store/src/", "codegen/tables/Tables.sol")).toMatchInlineSnapshot(
'"@latticexyz/store/src/codegen/tables/Tables.sol"',
);
expect(renderImportPath("@latticexyz/store/src", "./codegen/tables/Tables.sol")).toMatchInlineSnapshot(
'"@latticexyz/store/src/codegen/tables/Tables.sol"',
);
expect(renderImportPath("@latticexyz/store/src/", "./codegen/tables/Tables.sol")).toMatchInlineSnapshot(
'"@latticexyz/store/src/codegen/tables/Tables.sol"',
);
expect(renderImportPath("@latticexyz/store/src", "../test/codegen/common.sol")).toMatchInlineSnapshot(
'"@latticexyz/store/test/codegen/common.sol"',
);
expect(renderImportPath("@latticexyz/store/src/", "../test/codegen/common.sol")).toMatchInlineSnapshot(
'"@latticexyz/store/test/codegen/common.sol"',
);
});

it("returns valid path for relative imports", () => {
expect(renderImportPath(".")).toMatchInlineSnapshot('"."');
expect(renderImportPath("./")).toMatchInlineSnapshot('"."');
expect(renderImportPath(".", "IStore.sol")).toMatchInlineSnapshot('"./IStore.sol"');
expect(renderImportPath("./", "IStore.sol")).toMatchInlineSnapshot('"./IStore.sol"');
expect(renderImportPath("./src")).toMatchInlineSnapshot('"./src"');
expect(renderImportPath("./src/")).toMatchInlineSnapshot('"./src"');
expect(renderImportPath("./src", "IStore.sol")).toMatchInlineSnapshot('"./src/IStore.sol"');
expect(renderImportPath("../test/", "IStore.sol")).toMatchInlineSnapshot('"../test/IStore.sol"');
expect(renderImportPath("../test")).toMatchInlineSnapshot('"../test"');
expect(renderImportPath("../test/")).toMatchInlineSnapshot('"../test"');
expect(renderImportPath("../test", "IStore.sol")).toMatchInlineSnapshot('"../test/IStore.sol"');
expect(renderImportPath("../test/", "IStore.sol")).toMatchInlineSnapshot('"../test/IStore.sol"');
expect(renderImportPath(".", "codegen/tables/Tables.sol")).toMatchInlineSnapshot('"./codegen/tables/Tables.sol"');
expect(renderImportPath("./", "codegen/tables/Tables.sol")).toMatchInlineSnapshot('"./codegen/tables/Tables.sol"');
expect(renderImportPath(".", "./codegen/tables/Tables.sol")).toMatchInlineSnapshot('"./codegen/tables/Tables.sol"');
expect(renderImportPath("./", "./codegen/tables/Tables.sol")).toMatchInlineSnapshot(
'"./codegen/tables/Tables.sol"',
);
expect(renderImportPath(".", "../test/codegen/common.sol")).toMatchInlineSnapshot('"../test/codegen/common.sol"');
expect(renderImportPath("./", "../test/codegen/common.sol")).toMatchInlineSnapshot('"../test/codegen/common.sol"');
expect(renderImportPath("./src", "codegen/tables/Tables.sol")).toMatchInlineSnapshot(
'"./src/codegen/tables/Tables.sol"',
);
expect(renderImportPath("./src/", "codegen/tables/Tables.sol")).toMatchInlineSnapshot(
'"./src/codegen/tables/Tables.sol"',
);
expect(renderImportPath("./src", "./codegen/tables/Tables.sol")).toMatchInlineSnapshot(
'"./src/codegen/tables/Tables.sol"',
);
expect(renderImportPath("./src/", "./codegen/tables/Tables.sol")).toMatchInlineSnapshot(
'"./src/codegen/tables/Tables.sol"',
);
expect(renderImportPath("./src", "../test/codegen/common.sol")).toMatchInlineSnapshot(
'"./test/codegen/common.sol"',
);
expect(renderImportPath("./src/", "../test/codegen/common.sol")).toMatchInlineSnapshot(
'"./test/codegen/common.sol"',
);
});

it("normalizes to POSIX paths", () => {
expect(renderImportPath("C:\\src")).toMatchInlineSnapshot('"C:/src"');
expect(renderImportPath("C:\\src\\")).toMatchInlineSnapshot('"C:/src"');
expect(renderImportPath("C:\\src", "./IStore.sol")).toMatchInlineSnapshot('"C:/src/IStore.sol"');
expect(renderImportPath("C:\\src\\", "./IStore.sol")).toMatchInlineSnapshot('"C:/src/IStore.sol"');
expect(renderImportPath("./src", ".\\IStore.sol")).toMatchInlineSnapshot('"./src/IStore.sol"');
expect(renderImportPath("./src", ".\\IStore.sol")).toMatchInlineSnapshot('"./src/IStore.sol"');
});
});
24 changes: 24 additions & 0 deletions packages/common/src/codegen/render-solidity/renderImportPath.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import path from "node:path";

// This will probably break for backslash-escaped POSIX paths,
// but we'll worry about that later.
function winToPosix(segment: string): string {
return segment.replaceAll(path.win32.sep, path.posix.sep);
}

export function renderImportPath(basePath: string, ...segments: readonly string[]): string {
// Solidity compiler expects POSIX paths
const fullPath = path.posix
.join(winToPosix(basePath), ...segments.map(winToPosix))
// remove trailing slash
.replace(/\/$/, "");

// `path.join` strips the leading `./`
// so if we started with a relative path, make it relative again
if (basePath.startsWith(".")) {
const relativePath = "./" + fullPath;
return relativePath.replace(/^(\.\/)+\./, ".");
}

return fullPath;
}
9 changes: 0 additions & 9 deletions packages/common/src/foundry/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,6 @@ export async function getRpcUrl(profile?: string): Promise<string> {
return (await getForgeConfig(profile)).eth_rpc_url || "http://127.0.0.1:8545";
}

/**
* Get the value of "remappings" from forge config
* @param profile The foundry profile to use
* @returns The array of remapping tuples `[from, to]`
*/
export async function getRemappings(profile?: string): Promise<[string, string][]> {
return (await getForgeConfig(profile)).remappings.map((line) => line.trim().split("=")) as [string, string][];
}

/**
* Execute a forge command
* @param args The arguments to pass to forge
Expand Down
2 changes: 1 addition & 1 deletion packages/store/mud.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { defineStore } from "./ts/config/v2/store";

export default defineStore({
codegen: {
storeImportPath: "../../",
storeImportPath: "./src",
Copy link
Member Author

Choose a reason for hiding this comment

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

fyi this is an internal config option

},
namespace: "store",
userTypes: {
Expand Down
2 changes: 1 addition & 1 deletion packages/store/src/codegen/tables/Hooks.sol

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/store/src/codegen/tables/ResourceIds.sol

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/store/src/codegen/tables/StoreHooks.sol

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions packages/store/src/codegen/tables/Tables.sol

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/store/test/codegen/tables/KeyEncoding.sol

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading