Add handler type-checking to mock indexer fixtures - #1470
Conversation
Lets a mock indexer config type-check TypeScript handler source against the `indexer` surface generated from its own config + schema. Each check runs in an isolated in-process TS program, so independent configs never collide on the global `envio` module augmentation. - napi: `generate_indexer_types` returns the generated `.envio/types.d.ts` for an inline config (reuses `ProjectTemplate::from_config`), mirroring `parse_config_yaml`'s inputs. - Core: `generateIndexerTypes` binding. - envio-tests: `TypeChecker.ts` type-checks handlers via the TS compiler API with the init-template tsconfig options; `MockIndexerConfig.parseYaml` gains `~handlers`, throwing on any type error. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QM68d5PWeGDSiVGh5xDiM4
The helper now builds a mock indexer fixture (config + optional handler type-check), so the `Config` suffix and `parseYaml` name no longer fit. Updates all call sites in envio-tests and test_codegen. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QM68d5PWeGDSiVGh5xDiM4
📝 WalkthroughWalkthroughThe PR adds YAML-based generation of ChangesYAML indexer types and fixture validation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Test
participant InternalTestIndexer
participant Core
participant NAPI
participant ProjectTemplate
participant TypeChecker
Test->>InternalTestIndexer: fromUserApi(configYaml, handlers)
InternalTestIndexer->>Core: parseConfigYaml(configYaml)
InternalTestIndexer->>Core: generateIndexerTypes(configYaml)
Core->>NAPI: generateIndexerTypes(yaml, options)
NAPI->>ProjectTemplate: build generated template
ProjectTemplate-->>NAPI: indexer_types_dts()
NAPI-->>Core: types.d.ts content
InternalTestIndexer->>TypeChecker: checkHandlerTypes(typesDts, handlers)
TypeChecker-->>InternalTestIndexer: diagnostics
InternalTestIndexer-->>Test: config or handler type errors
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/envio/src/Core.res (1)
224-225: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove comments that restate the API or caller flow.
packages/envio/src/Core.res#L224-L225: remove the generated-types API description.packages/envio-tests/test/helpers/MockIndexerFixture.res#L1-L2: retain only a non-obvious compiler-API constraint, if needed.packages/envio-tests/test/helpers/MockIndexerFixture.res#L11-L13: remove thefromYamlbehavior narration.scenarios/test_codegen/test/helpers/MockIndexer.res#L410-L410: remove the fixture-provenance pointer.As per coding guidelines, comments must not restate what code does or point to where values are defined.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/envio/src/Core.res` around lines 224 - 225, Remove the API/caller-flow comment in packages/envio/src/Core.res at lines 224-225; in packages/envio-tests/test/helpers/MockIndexerFixture.res at lines 1-2, retain only any genuinely non-obvious compiler-API constraint; remove the fromYaml behavior narration at lines 11-13; and remove the fixture-provenance pointer in scenarios/test_codegen/test/helpers/MockIndexer.res at line 410. Keep code unchanged and retain comments only when they document non-obvious constraints.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/envio/src/Core.res`:
- Around line 224-225: Remove the API/caller-flow comment in
packages/envio/src/Core.res at lines 224-225; in
packages/envio-tests/test/helpers/MockIndexerFixture.res at lines 1-2, retain
only any genuinely non-obvious compiler-API constraint; remove the fromYaml
behavior narration at lines 11-13; and remove the fixture-provenance pointer in
scenarios/test_codegen/test/helpers/MockIndexer.res at line 410. Keep code
unchanged and retain comments only when they document non-obvious constraints.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 96ede47b-d5db-4f20-93c0-b616b35624c4
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
packages/cli/src/hbs_templating/codegen_templates.rspackages/cli/src/napi.rspackages/envio-tests/package.jsonpackages/envio-tests/test/ConfigYaml_test.respackages/envio-tests/test/MockIndexerHandlers_test.respackages/envio-tests/test/helpers/MockIndexerConfig.respackages/envio-tests/test/helpers/MockIndexerFixture.respackages/envio-tests/test/helpers/TypeChecker.tspackages/envio-tests/test/lib_tests/ColumnNameFormat_test.respackages/envio/src/Core.resscenarios/test_codegen/test/YamlConfigIndexer_test.resscenarios/test_codegen/test/helpers/MockIndexer.res
💤 Files with no reviewable changes (1)
- packages/envio-tests/test/helpers/MockIndexerConfig.res
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7799f7cf7a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| [handlersPath, handlers], | ||
| ]); | ||
|
|
||
| const host = ts.createCompilerHost(compilerOptions, true); |
There was a problem hiding this comment.
createCompilerHost leaves getCurrentDirectory() as process.cwd(), so the types: ["node"] lookup is based on whichever package launched the test rather than helpersDir/envio-tests. When MockIndexerFixture.fromYaml(~handlers=...) is called from a package that does not also expose @types/node, the fixture reports a global TS2688 even though envio-tests declares that dependency; importing checkHandlerTypes from the repo root reproduces this. Set the compiler host current directory to helpersDir (or parse a tsconfig rooted there) before creating the program.
Useful? React with 👍 / 👎.
…gYaml - MockIndexerFixture -> InternalTestIndexer, fromYaml -> fromUserApi (it builds a fixture through the user-facing public boundary). - Make the config YAML a labeled ~configYaml argument at all call sites. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QM68d5PWeGDSiVGh5xDiM4
`createCompilerHost`'s current directory defaults to `process.cwd()`, so `types: ["node"]` resolution depended on the launching package rather than envio-tests — a caller without `@types/node` reachable from its cwd got a spurious global TS2688. Point the host at helpersDir so type-root lookup is stable across packages. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QM68d5PWeGDSiVGh5xDiM4
…andlers-plan-shi26i # Conflicts: # packages/envio-tests/test/ConfigYaml_test.res
Matches the module it exercises (InternalTestIndexer.fromUserApi). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QM68d5PWeGDSiVGh5xDiM4
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scenarios/test_codegen/test/helpers/MockIndexer.res (1)
410-410: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the parser-origin comment.
Line 410 only identifies where callers obtain
customConfig; line 411 already documents its non-obvious default behavior.Proposed cleanup
- // A config parsed through the user-facing pipeline (InternalTestIndexer.fromUserApi). // Defaults to the generated project config.As per coding guidelines, “Don't write a comment that restates what the code already says … which callers use a value, history of a refactor, or pointers to where something is ‘now defined’.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scenarios/test_codegen/test/helpers/MockIndexer.res` at line 410, Remove the parser-origin comment immediately above the customConfig setup; retain the existing comment documenting its non-obvious default behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@scenarios/test_codegen/test/helpers/MockIndexer.res`:
- Line 410: Remove the parser-origin comment immediately above the customConfig
setup; retain the existing comment documenting its non-obvious default behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d5ac382a-70cd-45fa-a0ae-3e3c5f702f0e
📒 Files selected for processing (10)
packages/cli/src/hbs_templating/codegen_templates.rspackages/envio-tests/test/FromUserApi_test.respackages/envio-tests/test/MockIndexerHandlers_test.respackages/envio-tests/test/helpers/InternalTestIndexer.respackages/envio-tests/test/helpers/TypeChecker.tspackages/envio-tests/test/lib_tests/ColumnNameFormat_test.respackages/envio/src/Core.resscenarios/test_codegen/test/YamlConfigIndexer_test.resscenarios/test_codegen/test/helpers/MockIndexer.resscenarios/test_codegen/test/lib_tests/ClickHouse_test.res
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/envio-tests/test/MockIndexerHandlers_test.res
- packages/envio-tests/test/helpers/TypeChecker.ts
- packages/envio/src/Core.res
The comment only named which callers supply the value; the line below already documents its non-obvious default. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QM68d5PWeGDSiVGh5xDiM4
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 789d2b09ea
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ~handlers=` | ||
| import { indexer } from "envio"; | ||
| import type { Address } from "envio"; | ||
| import { expectType, type TypeEqual } from "ts-expect"; |
There was a problem hiding this comment.
When this positive handler fixture is type-checked, this import is resolved by the new checkHandlerTypes program; the locked ts-expect@1.3.0 package points its typings at dist/index.d.ts, but the installed package contents do not include dist, so the fixture reports TS2307: Cannot find module 'ts-expect' before it ever validates the generated indexer types. Avoid this dependency in the inline handler or switch to an assertion package whose declarations are actually published.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This looks like a false positive. ts-expect@1.3.0 does ship its declarations — the installed package contains node_modules/ts-expect/dist/index.d.ts (which is exactly what "typings" points to), so the import resolves. The positive fixture confirms it: it type-checks to zero diagnostics, and a TS2307 on this import would surface as a handler-file diagnostic and make fromUserApi throw, failing the test. No change needed.
Generated by Claude Code
Collapse the `parse_config_yaml` + `generate_indexer_types` napi exports into a single `from_user_api` that parses the inline config once and, when asked, returns the generated `.envio/types.d.ts` alongside the public config JSON — removing the double-parse and the risk of the runtime config drifting from the types. Drop the unused `isRescript` option from the test helper and napi surface. Narrow the handler type-check to diagnostics inferred in the handler source (plus global config errors); the generated `.d.ts` is trusted, not re-checked. Assert the TypeChecker's TypeScript version matches the init-template pin so the two can't silently diverge, and cache the unchanged declaration graph + reuse the prior program across fixtures so each check no longer re-parses `envio` and `@types/node`. Assert the exact diagnostic in the negative handler test, and rename the config-validation test to UserApiValidation_test. Claude-Session: https://claude.ai/code/session_018jWRbYWgh5DQ74GF6NDdu2 Co-authored-by: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 434101769d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }; | ||
|
|
||
| const typesPath = path.join(helpersDir, "__mock_indexer_types.d.ts"); | ||
| const handlersPath = path.join(helpersDir, "__mock_indexer_handlers.ts"); |
There was a problem hiding this comment.
When ~handlers is used from another test package, every import in the virtual handler is resolved as if the file lived under packages/envio-tests/test/helpers. That makes valid project handlers fail with TS2307 for dependencies that are declared by the caller but not by envio-tests itself (for example scenarios/test_codegen declares viem and its real handlers import it, but this helper would search from envio-tests instead of that scenario). Root the virtual handler in the caller/project directory, or accept a base directory for module resolution, so the fixture matches production handler compilation.
Useful? React with 👍 / 👎.
|
|
||
| type fromUserApiResult = { | ||
| config: string, | ||
| indexerTypes: Null.t<string>, |
There was a problem hiding this comment.
When withIndexerTypes is false, the native object omits indexerTypes rather than setting it to null, but this binding exposes it as Null.t<string>. Any ReScript caller that converts the field with Null.toOption will treat undefined as Some(undefined) (instead of None) and can then pass an undefined string into the type checker or other string consumers. Use a binding that handles both null and undefined, or make the Rust side always populate the field with null.
Useful? React with 👍 / 👎.
2 participants
What
Lets an internal test-indexer fixture type-check TypeScript handler source (
import { indexer } from "envio"; indexer.onEvent(...)) against theindexersurface generated from its own inlineconfig.yaml+schema.graphql. This is the type-test half of the tooling; running the handlers is intentionally out of scope for now.A handler type error throws with the compiler diagnostics.
How
from_user_api— parses an inline config the way a user's project would (no filesystem/env access) and, whenwith_indexer_typesis set, also returns the generated.envio/types.d.ts(the same TypeScript production codegen writes, viaProjectTemplate::from_config) from the same single parse. Returns{ config, indexer_types }. Surfaced in ReScript asCore.fromUserApi.TypeChecker.ts— type-checks the handlers in an isolated, in-processts.Programusing the same compiler options as the init-templatetsconfig.json, pinned to the same TypeScript version the templates pin. The generateddeclare module "envio"augmentation is injected as a virtual file, so each fixture bindsindexerto its own config for that program only — independent fixtures never collide on the global augmentation. Theenvio+ lib.d.tsgraph is parsed once and itsSourceFiles (plus the prior program structure) are reused across calls, so only the two per-fixture virtual files re-parse.InternalTestIndexer.fromUserApi— the fixture builder; gains an optional~handlers(throws on any handler type error).configYamlis a labeled argument.Tests
MockIndexerHandlers_test— a clean handler set type-checks; a nonexistent event name throws the exact diagnosticType '"Nonexistent"' is not assignable to type '"Transfer"'.UserApiValidation_test(config parsing/validation) andColumnNameFormat_testroute throughInternalTestIndexer.fromUserApi; cross-packagetest_codegen(YamlConfigIndexer_test,ClickHouse_test) too.envio_types_dtssnapshots (evm/fuel/svm) unchanged.Summary by CodeRabbit
New Features
Bug Fixes
Tests
🤖 Generated with Claude Code
https://claude.ai/code/session_01QM68d5PWeGDSiVGh5xDiM4