fix(types): Repair generate-types after Cluster export removal#187
Merged
Conversation
The Generate TypeScript Types workflow has been failing on master since PR #181 merged because two recent PRs collided: - PR #182 (b799858) removed `Cluster` from `lib/clusters/index.js` exports, on the rationale that the base class doesn't belong in a file dedicated to named cluster exports. - PR #181 introduced `scripts/generate-types.mts`, which looked up the shared `Cluster.clusters` static registry via `clustersModule.default.Cluster.clusters[ID]`. After both landed, `clustersModule.default.Cluster` is `undefined` and the script crashes. `Cluster.clusters` is a singleton attached to the class itself, so reaching it through the top-level `index.js` entry (already imported as `topModule`) yields the same registry without re-introducing the removed re-export. Also backports a `BoundClusterInterface` type to the generator template that PR 05332d1 added directly to the generated `index.d.ts` but forgot to add to the template. Without this, the next regeneration would silently drop a public type from the package's type surface. Verified locally on Node 24: - `npm run generate-types` succeeds - `npx tsc --noEmit index.d.ts` is clean - Regenerated `index.d.ts` matches the committed file (no drift) - `npm run lint` and `npm test` pass
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes the TypeScript type-generation script after Cluster stopped being re-exported from lib/clusters/index.js, restoring the “Generate TypeScript Types” workflow on master. Also aligns the declaration template with the committed index.d.ts so future regenerations don’t drop the public BoundClusterInterface type.
Changes:
- Update
scripts/generate-types.mtsto access theCluster.clustersregistry via the top-levelindex.jsexport (topModule.Cluster) instead oflib/clusters/index.js. - Add the missing
BoundClusterInterfacetype toscripts/template.d.ts.txtto prevent drift on regeneration.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| scripts/generate-types.mts | Fixes the registry lookup path so type generation no longer crashes when Cluster isn’t exported from lib/clusters/index.js. |
| scripts/template.d.ts.txt | Adds BoundClusterInterface to the template to keep generated index.d.ts stable across runs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
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.
The Generate TypeScript Types workflow has been failing on
mastersince #181 merged (failing run).Root cause
Two recently-merged PRs interact badly:
Clusterfromlib/clusters/index.jsexports because the base class doesn't belong in a file dedicated to named cluster exports.scripts/generate-types.mts, which on line 83 reaches the shared registry via:Both PRs were green on their own branches because they didn't yet see each other's state. After the #181 merge,
clustersModule.default.Clusterisundefined->TypeError: Cannot read properties of undefined (reading 'clusters').Fix
Cluster.clustersis a singleton static registry attached to the class itself; the path you reach the class through doesn't matter. The script already imports the top-level package entry astopModulefortopModule.ZCLDataTypes, andindex.jsstill exportsCluster, so the lookup just needs to go through there:This avoids re-introducing the export PR #182 deliberately removed.
Drive-by template fix
While regenerating
index.d.tslocally to verify, noticed the script's output didn't match the committed file: commit 05332d1 ("feat(types): add type definition for BoundCluster") added theBoundClusterInterfaceutility type directly toindex.d.tsbut forgot to add it toscripts/template.d.ts.txt. Without backporting it, the next workflow-driven regeneration would silently drop that public type. Added it to the template so the generated output stays stable.Verification
On Node 24 locally:
npm run generate-typesnpx tsc --noEmit index.d.tsindex.d.tsvs committednpm run lintnpm testOnce this lands, the Generate TypeScript Types workflow should go green on master and the auto-commit step becomes a no-op (since the template now reflects the committed
index.d.ts).