Phase 0: compiler and CI gates - #344
Merged
Merged
Conversation
- Add a `typecheck` script: runs codegen (src/types is gitignored, so a type-check that does not regenerate it checks a phantom) then `tsc --noEmit -p tsconfig.test.json` (the test config, so the gate also covers tests/**). - Pin `skipLibCheck: true` in tsconfig.json, with the reason recorded inline: @subql/types' global.d.ts and tests/unit/globals.d.ts both declare `api`/`unsafeApi`/`logger` as SubQuery runtime globals, which without it is a hard TS2451 redeclaration conflict, plus a handful of node_modules .d.ts files using syntax this TypeScript version cannot parse. Every suppressed error is in node_modules; project code type-checks clean without it. - Add a CI step running `yarn typecheck`, after `build src` and before `lint`. There was previously no type-check gate anywhere in the build or CI.
Chain storage reads were not type-augmented, so most were `Codec` and every field access on one was unchecked. - Replace `@polkadot/api-augment` (the generic Substrate kitchensink augmentation) with polymesh-types' `polkadot/augment-api` / `polkadot/types-lookup`, imported once from src/index.ts. This is a replacement, not an addition: loading both makes import order silently decide which chain ~161 shared members describe. - Import `@polkadot/types-augment` directly for the runtime type registry side effect `@polkadot/api-augment` used to pull in (`@polkadot/api` does not load it on its own). - Pin `@polymeshassociation/polymesh-types` to an exact 7.4.0 rather than a caret range: the augmentation reflects one metadata snapshot, and a floating range would let a legacy read start failing to compile on a patch bump with no other change. - Pin `moduleResolution: "node"` in tsconfig.json: `@polkadot/api-base` ships its storage declarations twice behind conditional exports, and polymesh-types augments the bare specifier one physical file resolves to. A mixed-mode build could augment one file while another config reads the other, silently losing the chain types. - Add a `legacyQuery` escape hatch (src/utils/legacyQuery.ts) for the two pre-7.x storage names the current metadata no longer carries — `multiSig.proposalDetail` and `multiSig.multiSigToIdentity`, renamed to `proposalStates`/`adminDid` at spec 7.0.0. It throws rather than returning `undefined` if the entry is genuinely absent, covered by a new unit test. polymesh-types augments one metadata snapshot while an indexer reads storage across every spec version the chain has ever had, so this escape hatch is structural, not a workaround. Its internal cast goes through `unknown` first: `@subql/node` bundles its own copy of `@polkadot/api-base` alongside the top-level one, so a direct cast fails under `subql build`'s ts-loader even though it type-checks fine under plain `tsc`. - Fix three real argument-type looseness sites the augmentation caught: `asset.customTypes` keys on `CustomAssetTypeId` (u32), and `asset.assetNames`/`fundingRound` key on a fixed-width codec — `.toU8a()` is the same bytes in both eras, so behaviour is unchanged. - Add an ESLint `no-restricted-imports` rule so `@polkadot/api-augment` cannot be silently reintroduced.
`project.ts` resolves each handler by name at runtime with no static link to its implementation, so a typo or a removed export is silently dropped rather than failing the build. `Suspended: ['handleBalanceSuspended']` named a function that was never exported anywhere in `src/` (defect A3) and nothing caught it. - Add scripts/check-handlers.ts: asserts every handler name project.ts references is exported from src/index.ts, exiting non-zero and listing the missing name(s) otherwise. - Wire it into CI (after typecheck, before lint) and into `prepack`. - Add scripts/**/* and project.ts to tsconfig.test.json's include so they are covered by `yarn typecheck` too — scripts/**/* already saw SubQuery's injected globals via tsconfig.json's `ts-node.files`, but nothing type-checked them until this script needed project.ts. - Resolve A3 by setting `Suspended` to `[]`, with a comment that the handler arrives with the POLYX ledger work (docs/implementation/02-polyx-ledger.md), which rewrites mapPolyxTransaction.ts wholesale — writing handleBalanceSuspended now means writing it twice.
`mapBlock` is only called from `handleEvent`, so a `Block` row exists only for a block that produced at least one handled event. Consumers have inferred indexer liveness from `MAX(blockId)` incorrectly — it can sit minutes behind the chain head while the indexer is perfectly current. Add a docstring to the `Block` entity in schema.graphql recording that the table is sparse and is not a freshness signal, and pointing at `_metadata.lastProcessedHeight` instead. Docstring only, no field changes.
prashantasdeveloper
force-pushed
the
redesign/00-compiler-ci
branch
from
September 7, 2026 06:52
9d40d4f to
cd695ef
Compare
|
F-OBrien
approved these changes
Sep 7, 2026
prashantasdeveloper
marked this pull request as ready for review
September 7, 2026 12:50
Contributor
Author
|
/fast-forward |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 0: Compiler and CI
PR 1 of 10 in the indexer redesign series. Based on
docs/architecture, targetsdocs/architecture.This PR adds the type-checking and build-time gates the project was missing, then uses them to catch and fix two real bugs.
What changed
A
typecheckscript, wired into CI. There was no type-check gate anywhere in the build or in CI —lintruns syntax-only rules, andsubql buildjust compiles. Addedyarn typecheck(regenerates the SubQuery-generated types undersrc/types, then runstsc --noEmit) and a CI step for it, right after the build step and before lint.Chain storage reads are now actually typed. The project imported
@polkadot/api-augment, which describes the generic Substrate "kitchensink" runtime, not Polymesh. Becauseapi.querycarries a fallback index signature, every chain storage read silently compiled to a bareCodecregardless — so a call likeapi.query.asset.assetNames(...)type-checked with zero information about its actual shape. Swapped it for the Polymesh-specific type augmentation from@polymeshassociation/polymesh-types. This isn't additive — loading both packages together makes import order silently decide which chain's types win for every member the two share, which is worse than no augmentation at all. Turning this on surfaced 5 real issues, all fixed here:multiSig.proposalDetail,multiSig.multiSigToIdentity) were renamed in a past chain upgrade and no longer exist in current metadata, so they don't type-check against the current augmentation even though the code still needs to read them on old blocks. Added a smalllegacyQueryhelper that looks the entry up by name at runtime and throws clearly if it's genuinely missing, instead of silently returningundefined.@polkadot/api-augmentcan't be quietly reintroduced later.The build now fails if
project.tsregisters a handler that doesn't exist.project.tsmaps chain events to handler function names as plain strings, with nothing checking those names against what's actually exported. One entry (Suspended: ['handleBalanceSuspended']) named a function that was never written anywhere in the codebase — so that event has been silently unhandled. Added a script that asserts every handler name inproject.tsis exported from the code, wired into CI and into the package'sprepackstep. The broken registration itself is fixed by removing it (set to[]) rather than writing a throwaway handler, since that event's real handler is coming as part of a larger, already-planned rewrite of how POLYX balance changes are tracked.Documented that the
Blocktable is sparse. ABlockrow is only written for a block that produced at least one handled event, not for every block. That means the highestBlockid is not a reliable way to check how far behind the indexer is — it can look stale by minutes while the indexer is fully caught up. Added a note directly on theBlocktype in the schema so this is visible to anyone querying it, instead of being a fact only visible in code.Consumer impact
None. No GraphQL schema field changes (the
Blocknote is a description-only addition), no runtime behavior changes, no query surface changes.Verification
All green: 201 unit tests passing, 0 type errors, and the handler check confirms all 157 registered handlers resolve to real exports.