-
Notifications
You must be signed in to change notification settings - Fork 194
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
Changes from all commits
1124267
f6ba539
154778c
f074cbd
52749bd
5114b26
0f41a03
68ec863
a6f9179
71ed19d
ec3c214
d85934e
82e9004
dcd6bc6
07b0247
e9071c7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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.
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.
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"; | ||
|
||
|
@@ -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" }, | ||
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" }, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. previously, codegen would parse the file at 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 |
||
ResourceId: { filePath: "@latticexyz/store/src/ResourceId.sol", type: "bytes32" }, | ||
}, | ||
tables: { | ||
|
@@ -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 }); |
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"'); | ||
}); | ||
}); |
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; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,7 @@ import { defineStore } from "./ts/config/v2/store"; | |
|
||
export default defineStore({ | ||
codegen: { | ||
storeImportPath: "../../", | ||
storeImportPath: "./src", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fyi this is an internal config option |
||
}, | ||
namespace: "store", | ||
userTypes: { | ||
|
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.
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.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
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 themud.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 placethis was an inconsistency/edge case that should now be more consistent and predictable and enables better resolving of user types from within namespaced directories