feat(types): extend ABI tooling for schema generation #491
Conversation
WalkthroughIntroduces an ABI-centric workflow: a CLI to collect, deduplicate, and output a consolidated compiled-abi.json and optional TypeScript types, plus extensive TypeScript type utilities and tests for ABI-derived models/actions. Documentation and examples are updated accordingly. A changeset records a patch release for the new tooling. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant CLI as compile-abi.ts (CLI)
participant FS as File System
participant MF as Manifest & Targets
participant GEN as TS Generator
U->>CLI: Execute with args (--output, --generate-types)
CLI->>CLI: parseArgs(argv)
CLI->>CLI: resolveOutputPaths(option?)
CLI->>FS: ensureDirectory(paths.json parent)
CLI->>MF: Read manifest and discover target JSONs (recursive)
CLI->>CLI: Collect ABIs from world/contracts/targets
CLI->>CLI: Deduplicate by (type,name,interface_name)
CLI->>FS: Write compiled-abi.json (merged)
alt --generate-types
CLI->>FS: ensureDirectory(paths.ts parent)
CLI->>GEN: generateAbiTypes(paths)
GEN->>FS: Write compiled-abi.ts
end
CLI-->>U: Log counts, paths, warnings (duplicates)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (9)
packages/core/src/types/example-usage.ts (1)
9-11: Broken dev import will likely fail builds; switch to a safe placeholder or documented path.Importing from "./nums_dev" is repo-local and may not exist. For examples, prefer the documented compiled-abi path or a declared placeholder to avoid compile-time resolution errors.
Apply this diff:
-// import { compiledAbi } from "../../../../worlds/dojo-starter/compiled-abi"; -import { compiledAbi } from "./nums_dev"; +// Prefer importing the generated file in real usage: +// import { compiledAbi } from "./compiled-abi"; +// For example code in the package, declare a placeholder to avoid module resolution issues: +declare const compiledAbi: { abi: readonly any[] };packages/core/src/types/README.md (1)
13-16: Clarify extensionless output behavior.Docs say TS is emitted “alongside the JSON with a .ts extension.” When the output path lacks .json, your implementation keeps JSON as-is and appends .ts for TS. Add a quick example to avoid confusion.
Example addition:
- --output artifacts/abi/compiled-abi → writes artifacts/abi/compiled-abi and artifacts/abi/compiled-abi.ts
packages/core/src/types/index.ts (1)
226-234: Tuple mapping may miss formats without spaces after commas.Cairo/ABI strings can be formatted as "(a,b)" or "(a, b)". Handle both.
Apply this diff:
-type MapTupleTypes< - T extends string, - ABI extends readonly any[] = never, -> = T extends `${infer First}, ${infer Rest}` - ? [MapCairoType<First, ABI>, ...MapTupleTypes<Rest, ABI>] - : T extends "" - ? [] - : [MapCairoType<T, ABI>]; +type MapTupleTypes< + T extends string, + ABI extends readonly any[] = never, +> = T extends `${infer First}, ${infer Rest}` + ? [MapCairoType<First, ABI>, ...MapTupleTypes<Rest, ABI>] + : T extends `${infer First},${infer Rest}` // handle no-space commas + ? [MapCairoType<First, ABI>, ...MapTupleTypes<Rest, ABI>] + : T extends "" + ? [] + : [MapCairoType<T, ABI>];packages/core/src/cli/compile-abi.ts (6)
56-71: Use DOJO_ROOT for default output path and accept directory-like --output valuesDefaulting to process.cwd() can surprise users when DOJO_ROOT is set. Also, treating a directory-like --output as a directory improves UX. The current dojoRoot parameter is otherwise unused.
Apply this diff:
export function resolveOutputPaths( - dojoRoot: string, - outputOption?: string -): OutputPaths { - const jsonPath = outputOption - ? isAbsolute(outputOption) - ? outputOption - : join(process.cwd(), outputOption) - : join(process.cwd(), "compiled-abi.json"); - - const tsPath = jsonPath.endsWith(".json") - ? `${jsonPath.slice(0, -5)}.ts` - : `${jsonPath}.ts`; - - return { json: jsonPath, ts: tsPath }; + dojoRoot: string, + outputOption?: string +): OutputPaths { + const resolved = outputOption + ? (isAbsolute(outputOption) ? outputOption : join(process.cwd(), outputOption)) + : join(dojoRoot, "compiled-abi.json"); + + // If the provided output looks like a directory, write the default filename inside it + const jsonPath = /[\/\\]$/.test(resolved) || !resolved.toLowerCase().endsWith(".json") + ? join(resolved, "compiled-abi.json") + : resolved; + + const tsPath = jsonPath.endsWith(".json") + ? `${jsonPath.slice(0, -5)}.ts` + : `${jsonPath}.ts`; + + return { json: jsonPath, ts: tsPath }; }
37-46: Types appear intended to be public but aren’t exportedIf these types are used by tests/consumers, export them; otherwise consider marking them internal via comment.
-type OutputPaths = { +export type OutputPaths = { json: string; ts: string; }; -type CollectOptions = { +export type CollectOptions = { generateTypes: boolean; outputPath?: string; };
225-230: Clarify duplicate reporting (duplicates vs total occurrences)Current count represents total occurrences. If the goal is to show how many were ignored, print both.
- if (Object.keys(duplicateCounts).length > 0) { - console.warn("! Duplicate ABI entries detected and ignored:"); - for (const [key, count] of Object.entries(duplicateCounts)) { - console.warn(` • ${key} (${count} occurrences)`); - } - } + if (Object.keys(duplicateCounts).length > 0) { + console.warn("! Duplicate ABI entries detected and ignored:"); + for (const [key, count] of Object.entries(duplicateCounts)) { + const duplicates = Math.max(0, count - 1); + console.warn(` • ${key} (${duplicates} duplicates, ${count} total occurrences)`); + } + }
3-9: Harden direct-execution check against symlinksComparing real paths avoids false negatives when invoked via symlinked paths.
import { readFileSync, writeFileSync, readdirSync, existsSync, - mkdirSync, + mkdirSync, + realpathSync, } from "fs"; @@ -const __filename = resolve(fileURLToPath(import.meta.url)); -const entryPoint = process.argv[1] ? resolve(process.argv[1]) : undefined; +const __filename = realpathSync(fileURLToPath(import.meta.url)); +const entryPoint = process.argv[1] ? realpathSync(process.argv[1]) : undefined;Also applies to: 313-316
51-55: Minor naming nit: avoid shadowing “path” identifierRenaming improves readability alongside imported path helpers.
-function ensureDirectory(path: string): void { - const directory = dirname(path); +function ensureDirectory(filePath: string): void { + const directory = dirname(filePath); mkdirSync(directory, { recursive: true }); }
140-151: Consider using the shared Manifest type (and schema) instead of a local aliasImporting the Manifest type (and parsing via the zod schema) prevents drift with schema changes.
Would you like a follow-up diff to replace the local Manifest type with:
- import type { Manifest } from "../manifest";
- and parse via the exported manifestSchema for runtime validation?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/core/src/cli/compile-abi.test.ts(1 hunks)packages/core/src/cli/compile-abi.ts(7 hunks)packages/core/src/types/README.md(1 hunks)packages/core/src/types/example-usage.ts(3 hunks)packages/core/src/types/index.test.ts(1 hunks)packages/core/src/types/index.ts(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
packages/core/src/cli/compile-abi.test.ts (1)
packages/core/src/cli/compile-abi.ts (1)
resolveOutputPaths(56-71)
packages/core/src/types/example-usage.ts (1)
packages/core/src/types/index.ts (4)
ExtractAbiTypes(502-508)ModelsFromAbi(514-514)GetModel(529-540)GetActionFunction(553-558)
packages/core/src/cli/compile-abi.ts (1)
packages/core/src/manifest/index.ts (1)
Manifest(105-105)
packages/core/src/types/index.ts (1)
packages/grpc/src/generated/types.ts (2)
Model(35-94)Model(1119-1119)
packages/core/src/types/index.test.ts (1)
packages/core/src/types/index.ts (6)
ExtractAbiTypes(502-508)ModelPathFromAbi(527-527)GetModel(529-540)ActionsFromAbi(516-516)GetActionFunction(553-558)ModelsFromAbi(514-514)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (5)
packages/core/src/cli/compile-abi.test.ts (2)
24-30: Absolute path behavior looks good.For an absolute JSON path, TS path derived via .json → .ts is correct and matches expectations.
32-41: Extensionless output behavior is correct.Leaving the JSON path as-is and appending .ts for the TS path is consistent and predictable.
packages/core/src/types/index.test.ts (1)
1-120: Strong, targeted type-level tests.Good coverage of structs, model paths/lookup, enums, and action function typing with expectTypeOf. Aligns with the new public type utilities.
packages/core/src/types/index.ts (1)
510-559: Public ABI type surface looks coherent.The model/action collections, path union, and granular getters (GetModel/GetActionFunction) are well-factored atop ExtractAbiTypes. Nice use of Simplify/UnionToIntersection to normalize mapped unions.
packages/core/src/cli/compile-abi.ts (1)
112-131: LGTM: Recursive JSON discovery is clean and efficientIterative descent with Dirent is correct and avoids extra stat calls.
| it("returns defaults when no output option provided", () => { | ||
| const paths = resolveOutputPaths(root); | ||
|
|
||
| expect(paths.json).toEqual(join(root, "compiled-abi.json")); | ||
| expect(paths.ts).toEqual(join(root, "compiled-abi.ts")); | ||
| }); |
There was a problem hiding this comment.
Tests assume dojoRoot-based resolution, but implementation uses process.cwd().
resolveOutputPaths currently ignores its dojoRoot parameter (see compile-abi.ts snippet) and resolves defaults/relative paths from process.cwd(). This will fail these tests and produce incorrect CLI behavior when invoked from a different CWD than the Dojo root. Resolve relative paths and defaults against dojoRoot.
Apply this patch in packages/core/src/cli/compile-abi.ts:
export function resolveOutputPaths(
dojoRoot: string,
outputOption?: string
): OutputPaths {
- const jsonPath = outputOption
- ? isAbsolute(outputOption)
- ? outputOption
- : join(process.cwd(), outputOption)
- : join(process.cwd(), "compiled-abi.json");
+ const jsonPath = outputOption
+ ? isAbsolute(outputOption)
+ ? outputOption
+ : join(dojoRoot, outputOption)
+ : join(dojoRoot, "compiled-abi.json");
const tsPath = jsonPath.endsWith(".json")
? `${jsonPath.slice(0, -5)}.ts`
: `${jsonPath}.ts`;
return { json: jsonPath, ts: tsPath };
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it("returns defaults when no output option provided", () => { | |
| const paths = resolveOutputPaths(root); | |
| expect(paths.json).toEqual(join(root, "compiled-abi.json")); | |
| expect(paths.ts).toEqual(join(root, "compiled-abi.ts")); | |
| }); | |
| export function resolveOutputPaths( | |
| dojoRoot: string, | |
| outputOption?: string | |
| ): OutputPaths { | |
| const jsonPath = outputOption | |
| ? isAbsolute(outputOption) | |
| ? outputOption | |
| : join(dojoRoot, outputOption) | |
| : join(dojoRoot, "compiled-abi.json"); | |
| const tsPath = jsonPath.endsWith(".json") | |
| ? `${jsonPath.slice(0, -5)}.ts` | |
| : `${jsonPath}.ts`; | |
| return { json: jsonPath, ts: tsPath }; | |
| } |
MartianGreed
force-pushed
the
feat/types
branch
from
1f9cbc0 to
ff3912c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/core/src/cli/compile-abi.ts (2)
73-106: Usage snippet hardcodes './compiled-abi'; mismatch when outputPath is customized.Derive the import path from the actual emitted .ts file so guidance is always correct.
Apply this diff:
-import { join, isAbsolute, dirname, resolve } from "path"; +import { join, isAbsolute, dirname, resolve, basename } from "path"; @@ function generateAbiTypes(paths: OutputPaths): void { const { json: inputPath, ts: outputPath } = paths; @@ - console.log(` -Usage in your code:`); - console.log(` -import { compiledAbi } from './compiled-abi';`); + const importPath = `./${basename(outputPath).replace(/\.ts$/, "")}`; + console.log(`\nUsage in your code:`); + console.log(`\nimport { compiledAbi } from '${importPath}';`);
232-246: Output payload shape is helpful; minor note on manifest typing.Including selected manifest fields is useful. Local Manifest type uses an index signature; if you want stronger guarantees, consider importing the zod‑derived Manifest type later.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.changeset/mighty-feet-move.md(1 hunks)packages/core/src/cli/compile-abi.test.ts(1 hunks)packages/core/src/cli/compile-abi.ts(7 hunks)packages/core/src/types/README.md(1 hunks)packages/core/src/types/example-usage.ts(3 hunks)packages/core/src/types/index.test.ts(1 hunks)packages/core/src/types/index.ts(4 hunks)
✅ Files skipped from review due to trivial changes (1)
- .changeset/mighty-feet-move.md
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/core/src/cli/compile-abi.test.ts
- packages/core/src/types/README.md
- packages/core/src/types/example-usage.ts
🧰 Additional context used
🧬 Code graph analysis (3)
packages/core/src/cli/compile-abi.ts (1)
packages/core/src/manifest/index.ts (1)
Manifest(105-105)
packages/core/src/types/index.test.ts (1)
packages/core/src/types/index.ts (6)
ExtractAbiTypes(502-508)ModelPathFromAbi(527-527)GetModel(529-540)ActionsFromAbi(516-516)GetActionFunction(553-558)ModelsFromAbi(514-514)
packages/core/src/types/index.ts (1)
packages/grpc/src/generated/types.ts (2)
Model(35-94)Model(1119-1119)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (14)
packages/core/src/types/index.test.ts (4)
11-66: Well-constructed ABI fixture (covers structs, nested structs, enum, function, interface).Good breadth for exercising the new type utilities. No concerns.
70-84: Structs-to-models mapping assertions look correct.Assertions match the MapCairoType mapping (felt252 -> string, u32/u16 -> number) and nested struct handling.
86-96: Model path union and lookup are accurately validated.The expected "demo-Player" | "demo-Position" union and GetModel resolution align with the new utilities.
98-119: Enums and action function typing checks are precise.Enum variants as a string union and the action function’s input/output types align with ExtractAbiTypes/ActionsFromAbi/GetActionFunction behavior.
packages/core/src/cli/compile-abi.ts (5)
112-130: Recursive JSON discovery is clean and robust.walkJsonFiles correctly handles nested directories via Dirent traversal.
196-231: Deduplication key and counts look solid.Composite key (type::name::interface_name) avoids collisions; count logic reports total occurrences. Good.
247-259: Useful post-write stats.Per-type counts aid observability. Nicely done.
267-311: CLI arg parsing covers common forms.--generate-types, --output , and --output=val are handled; unknowns are safely warned.
313-326: Direct-execution guard is correct for ESM.isDirectExecution check via resolved paths is accurate; error handling exits cleanly.
packages/core/src/types/index.ts (5)
235-244: Utility types added are standard and useful.Simplify and UnionToIntersection underpin your model/action shaping helpers. No issues.
289-297: Extending ExtractAbiTypesFromArray with models/actions is coherent.The new fields align with downstream helpers and tests.
450-470: Model extraction by namespace/model is precise.Pattern-matching
${Namespace}::models::${Model}and intersection merge produce the desired nested map. Good use of type-level plumbing.
471-495: Action extraction mirrors models cleanly.Actions are mapped under
${Namespace}::systems::actions::${Interface}with interface items wired to function signatures. Looks correct.
510-559: Public API surface is well factored.ModelsFromAbi, ActionsFromAbi, path unions, and Get* accessors provide ergonomic, composable entry points. Matches tests.
packages/core/src/cli/compile-abi.ts
Outdated
| export function resolveOutputPaths( | ||
| dojoRoot: string, | ||
| outputOption?: string | ||
| ): OutputPaths { | ||
| const jsonPath = outputOption | ||
| ? isAbsolute(outputOption) | ||
| ? outputOption | ||
| : join(process.cwd(), outputOption) | ||
| : join(process.cwd(), "compiled-abi.json"); | ||
|
|
||
| const tsPath = jsonPath.endsWith(".json") | ||
| ? `${jsonPath.slice(0, -5)}.ts` | ||
| : `${jsonPath}.ts`; | ||
|
|
||
| return { json: jsonPath, ts: tsPath }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
resolveOutputPaths ignores dojoRoot; defaults and relatives are anchored to CWD.
This can write outputs outside DOJO_ROOT when DOJO_ROOT != CWD. Anchor defaults and relative paths to dojoRoot, and ensure bare paths get a .json for consistency.
Apply this diff:
-export function resolveOutputPaths(
- dojoRoot: string,
- outputOption?: string
-): OutputPaths {
- const jsonPath = outputOption
- ? isAbsolute(outputOption)
- ? outputOption
- : join(process.cwd(), outputOption)
- : join(process.cwd(), "compiled-abi.json");
-
- const tsPath = jsonPath.endsWith(".json")
- ? `${jsonPath.slice(0, -5)}.ts`
- : `${jsonPath}.ts`;
-
- return { json: jsonPath, ts: tsPath };
-}
+export function resolveOutputPaths(
+ dojoRoot: string,
+ outputOption?: string
+): OutputPaths {
+ const baseRoot = dojoRoot || process.cwd();
+ const resolved = outputOption
+ ? isAbsolute(outputOption)
+ ? outputOption
+ : join(baseRoot, outputOption)
+ : join(baseRoot, "compiled-abi.json");
+
+ const jsonPath = resolved.endsWith(".json") ? resolved : `${resolved}.json`;
+ const tsPath = jsonPath.slice(0, -5) + ".ts";
+
+ return { json: jsonPath, ts: tsPath };
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function resolveOutputPaths( | |
| dojoRoot: string, | |
| outputOption?: string | |
| ): OutputPaths { | |
| const jsonPath = outputOption | |
| ? isAbsolute(outputOption) | |
| ? outputOption | |
| : join(process.cwd(), outputOption) | |
| : join(process.cwd(), "compiled-abi.json"); | |
| const tsPath = jsonPath.endsWith(".json") | |
| ? `${jsonPath.slice(0, -5)}.ts` | |
| : `${jsonPath}.ts`; | |
| return { json: jsonPath, ts: tsPath }; | |
| } | |
| export function resolveOutputPaths( | |
| dojoRoot: string, | |
| outputOption?: string | |
| ): OutputPaths { | |
| const baseRoot = dojoRoot || process.cwd(); | |
| const resolved = outputOption | |
| ? isAbsolute(outputOption) | |
| ? outputOption | |
| : join(baseRoot, outputOption) | |
| : join(baseRoot, "compiled-abi.json"); | |
| const jsonPath = resolved.endsWith(".json") ? resolved : `${resolved}.json`; | |
| const tsPath = jsonPath.slice(0, -5) + ".ts"; | |
| return { json: jsonPath, ts: tsPath }; | |
| } |
🤖 Prompt for AI Agents
In packages/core/src/cli/compile-abi.ts around lines 56 to 71,
resolveOutputPaths currently anchors defaults and relative outputOption paths to
process.cwd(), ignoring dojoRoot and allowing writes outside DOJO_ROOT; change
logic to anchor everything to dojoRoot: if outputOption is provided and
absolute, keep it; if provided and relative, join it with dojoRoot; if not
provided, use join(dojoRoot, "compiled-abi.json"); after resolving the path, if
it has no extension append ".json" to ensure consistency; compute ts path by
replacing a trailing ".json" with ".ts" or appending ".ts" when no ".json"
extension existed; return the { json, ts } paths.
ff3912c to
ce9c7e4
Compare
MartianGreed
force-pushed
the
feat/types
branch
from
ce9c7e4 to
b31aa38
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/types/example-usage.ts (1)
24-26: CLI name mismatch — update example to usecompile-abior addgenerate-abi-typesbinpackages/core/src/types/example-usage.ts (lines 24–26): example uses
npx generate-abi-types, but @dojoengine/core package.json exposes a bin"compile-abi": "./dist/cli/compile-abi.js". Either update the example to the actual CLI invocation (e.g.,npx compile-abi) or add a"generate-abi-types": "./dist/cli/compile-abi.js"bin alias in @dojoengine/core's package.json.
🧹 Nitpick comments (2)
packages/core/src/cli/compile-abi.ts (2)
91-99: Make the usage hint reflect the actual generated filename.The hint always shows ./compiled-abi, which can mislead when a custom output name is used. Use basename(outputPath).
- console.log(` -Usage in your code:`); - console.log(` -import { compiledAbi } from './compiled-abi';`); - console.log(`import { ExtractAbiTypes } from '@dojoengine/core';`); - console.log(` -type MyAbi = ExtractAbiTypes<typeof compiledAbi>;`); + const base = basename(outputPath, ".ts"); + const importPath = `./${base}`; + console.log(`\nUsage in your code:`); + console.log(`\nimport { compiledAbi } from '${importPath}';`); + console.log(`import { ExtractAbiTypes } from '@dojoengine/core';`); + console.log(`\ntype MyAbi = ExtractAbiTypes<typeof compiledAbi>;`);Add basename to imports:
- import { join, isAbsolute, dirname, resolve } from "path"; + import { join, isAbsolute, dirname, resolve, basename } from "path";
37-46: Consider exporting the new public-facing types.If consumers are expected to import these from the CLI module, export them.
-type OutputPaths = { +export type OutputPaths = { json: string; ts: string; }; -type CollectOptions = { +export type CollectOptions = { generateTypes: boolean; outputPath?: string; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.changeset/mighty-feet-move.md(1 hunks)packages/core/src/cli/compile-abi.ts(7 hunks)packages/core/src/types/README.md(1 hunks)packages/core/src/types/example-usage.ts(3 hunks)packages/core/src/types/index.test.ts(1 hunks)packages/core/src/types/index.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- .changeset/mighty-feet-move.md
- packages/core/src/types/index.test.ts
- packages/core/src/types/README.md
🧰 Additional context used
🧬 Code graph analysis (3)
packages/core/src/types/example-usage.ts (1)
packages/core/src/types/index.ts (4)
ExtractAbiTypes(502-508)ModelsFromAbi(514-514)GetModel(529-540)GetActionFunction(553-558)
packages/core/src/cli/compile-abi.ts (1)
packages/core/src/manifest/index.ts (1)
Manifest(105-105)
packages/core/src/types/index.ts (1)
packages/grpc/src/generated/types.ts (2)
Model(35-94)Model(1119-1119)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build-packages
- GitHub Check: build-examples
- GitHub Check: build
🔇 Additional comments (4)
packages/core/src/cli/compile-abi.ts (2)
193-221: LGTM on ABI dedup strategy and reporting.Composite key and stable sort produce deterministic, readable outputs.
56-68: Essential: Anchor outputs to DOJO_ROOT and ensure “.json” extension (repeat of earlier feedback).resolveOutputPaths still anchors to process.cwd() and accepts bare paths without appending .json. This can place artifacts outside DOJO_ROOT and produce json files without an extension.
Apply:
-export function resolveOutputPaths(outputOption?: string): OutputPaths { - const jsonPath = outputOption - ? isAbsolute(outputOption) - ? outputOption - : join(process.cwd(), outputOption) - : join(process.cwd(), "compiled-abi.json"); - - const tsPath = jsonPath.endsWith(".json") - ? `${jsonPath.slice(0, -5)}.ts` - : `${jsonPath}.ts`; - - return { json: jsonPath, ts: tsPath }; -} +export function resolveOutputPaths( + dojoRoot: string, + outputOption?: string +): OutputPaths { + const baseRoot = dojoRoot || process.cwd(); + const resolved = outputOption + ? isAbsolute(outputOption) + ? outputOption + : join(baseRoot, outputOption) + : join(baseRoot, "compiled-abi.json"); + + const jsonPath = resolved.endsWith(".json") ? resolved : `${resolved}.json`; + const tsPath = jsonPath.slice(0, -5) + ".ts"; + + return { json: jsonPath, ts: tsPath }; +}packages/core/src/types/example-usage.ts (1)
37-41: Nice use of new model/action helpers.Clear, ergonomic access via Schema and GetModel.
packages/core/src/types/index.ts (1)
450-559: Strong addition: models/actions extraction and path-based lookups.The UnionToIntersection + Simplify approach yields clean namespaces, and the GetModel/GetAction* surface is intuitive.
| const paths = resolveOutputPaths(options.outputPath); | ||
| ensureDirectory(paths.json); | ||
| writeFileSync(paths.json, JSON.stringify(output, null, 2)); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Wire resolveOutputPaths to DOJO_ROOT.
Pass dojoRoot so relative outputs are anchored correctly.
- const paths = resolveOutputPaths(options.outputPath);
+ const paths = resolveOutputPaths(dojoRoot, options.outputPath);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const paths = resolveOutputPaths(options.outputPath); | |
| ensureDirectory(paths.json); | |
| writeFileSync(paths.json, JSON.stringify(output, null, 2)); | |
| const paths = resolveOutputPaths(dojoRoot, options.outputPath); | |
| ensureDirectory(paths.json); | |
| writeFileSync(paths.json, JSON.stringify(output, null, 2)); |
🤖 Prompt for AI Agents
In packages/core/src/cli/compile-abi.ts around lines 240 to 242,
resolveOutputPaths is being called without the Dojo root so relative output
paths may be incorrect; update the call to pass the dojoRoot (e.g.,
resolveOutputPaths(options.outputPath, options.dojoRoot) or the local dojoRoot
variable) so outputs are anchored to DOJO_ROOT, and adjust any
resolveOutputPaths signature/usage accordingly to accept and use the dojoRoot
when building paths.
| compiledAbi, | ||
| } from "../../../../worlds/dojo-starter/compiled-abi"; | ||
| // import { compiledAbi } from "../../../../worlds/dojo-starter/compiled-abi"; | ||
| import { compiledAbi } from "./nums_dev"; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify: Is ./nums_dev present? Prefer a type-only fallback to avoid build breaks.
If nums_dev isn’t committed, this import will break builds. Suggest keeping example type-only by declaring compiledAbi instead of importing.
Run to confirm presence:
If missing, apply:
- import { compiledAbi } from "./nums_dev";
+// Replace with your generated file (e.g., "../../../../worlds/dojo-starter/compiled-abi")
+// import { compiledAbi } from "../../../../worlds/dojo-starter/compiled-abi";
+declare const compiledAbi: { abi: readonly any[] };🏁 Script executed:
#!/bin/bash
# Check for nums_dev in the repo
fd -H -a -t f 'nums_dev\.(ts|tsx|js|mjs|cjs|json)'Length of output: 0
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Show file (if present): packages/core/src/types/example-usage.ts ==="
if [ -f packages/core/src/types/example-usage.ts ]; then
sed -n '1,240p' packages/core/src/types/example-usage.ts
else
echo "FILE MISSING"
fi
echo
echo "=== ripgrep for 'nums_dev' ==="
rg -n --hidden -S 'nums_dev' || echo "no matches"
echo
echo "=== ripgrep for 'compiledAbi' ==="
rg -n --hidden -S 'compiledAbi' || echo "no matches"Length of output: 5608
Remove broken import or provide a type-only fallback
packages/core/src/types/example-usage.ts:10 — ./nums_dev is not present in the repo; importing it will break builds. Replace the import with a type-only declaration or point to the generated compiled-abi file:
- import { compiledAbi } from "./nums_dev";
+// Replace with your generated file (e.g., "../../../../worlds/dojo-starter/compiled-abi")
+// import { compiledAbi } from "../../../../worlds/dojo-starter/compiled-abi";
+declare const compiledAbi: { abi: readonly any[] };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { compiledAbi } from "./nums_dev"; | |
| // Replace with your generated file (e.g., "../../../../worlds/dojo-starter/compiled-abi") | |
| // import { compiledAbi } from "../../../../worlds/dojo-starter/compiled-abi"; | |
| declare const compiledAbi: { abi: readonly any[] }; |
🤖 Prompt for AI Agents
In packages/core/src/types/example-usage.ts around line 10, the import of
"./nums_dev" is broken because that module doesn't exist in the repo; remove the
runtime import and either replace it with a type-only import (e.g., import type
{ compiledAbi } from "./nums_dev";) if you only need types, or point the import
to the correct generated compiled-abi file path; ensure the import uses
type-only syntax when appropriate or update the path to the actual generated
artifact so builds no longer fail.
Closes #
Introduced changes
Checklist
Summary by CodeRabbit
New Features
Improvements
Documentation
Tests
Chores