chore(layout-engine,super-editor): delete .d.ts shadows + add divergence gate (SD-2922) - #3108
Conversation
These hand-written declaration files lived next to .ts source with no generator producing them, so they drifted. The audit found 14 of 39 sibling pairs disagreed with their source; some declared functions and constants the source did not contain at all (style-engine declared resolveStyle, resolveNumbering, resolveTableCellStyle - none existed). TypeScript prefers a sibling .d.ts over inferring from the .ts source, so consumers were reading fictional APIs and would have shipped imports that returned undefined at runtime. Removing the shadows; TypeScript infers the correct types from the .ts sources directly. Workspace typecheck passes. The collaboration-yjs sibling .d.ts files are intentionally kept: tsup's --dts generator depends on them as compilation leaves and the package breaks without them. Reconfiguring tsup is a separate follow-up.
Add scripts/check-dts-shadows.mjs as a CI gate that fails if any .d.ts file lives next to a .ts file under packages/. Wire it into lefthook's pre-commit and into ci-superdoc.yml so the regression is caught both locally and on PRs. The collaboration-yjs prefix is allowlisted because tsup's --dts generator currently relies on those shadow files. That coupling is documented in the script header and tracked as a follow-up on SD-2922. The allowlist is intentionally a hard-coded prefix list, not a glob, to keep additions explicit.
![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: 73e52d22c2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const rel = relative(ROOT, file); | ||
| if (ALLOWLIST_PREFIXES.some((p) => rel.startsWith(p))) continue; |
There was a problem hiding this comment.
check-dts-shadows compares path.relative() output against a hardcoded forward-slash prefix, but relative() returns backslash-separated paths on Windows. In that environment, files under packages/collaboration-yjs/ no longer match the allowlist and are reported as violations, so the new gate fails despite the documented exception. This affects Windows contributors running the pre-commit hook (and any Windows CI jobs) until the path is normalized before startsWith.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
fixed in 83872b4 - normalized path separators (split on path.sep, join on '/') before the allowlist prefix check, so the gate behaves the same on Windows.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
The previous step inside ci-superdoc.yml's build job was gated by detect's path filter, which intentionally excludes some package paths (react, esign, sdk, template-builder, ai). A PR adding foo.d.ts beside foo.ts in any of those would have bypassed the gate on CI - lefthook catches it locally, but CI did not enforce. Move the check into its own workflow that runs on every pull_request and merge_group, so the rule's scope (all of packages/) matches the CI coverage. Keeps ci-superdoc.yml unchanged in scope and follows the per-concern workflow pattern the repo already uses.
…ge (SD-2922) Two changes: - Failure message now spells out the two valid actions: delete the .d.ts, or allowlist the package with a reason if its declaration build needs source-side shadows. The previous "delete it" wording invited an agent to extend the gate's intent into "delete every .d.ts I see". - Header AIDEV-NOTE adds the gate's scope boundary (only src/foo.d.ts + src/foo.ts pairs; never dist/ or generated output) and records that the one adjacent danger case (package.json types pointing at src/*.d.ts) was manually verified empty at SD-2922 time, with audit guidance if it ever reappears. No behavior change.
…ck (SD-2922) path.relative() returns backslash-separated paths on Windows, so the forward-slash ALLOWLIST_PREFIXES never matched there - collaboration-yjs files would be falsely flagged as violations on Windows runs of the lefthook pre-commit hook. Normalize via sep -> '/' before the prefix check so the allowlist behaves the same on every platform.
|
🎉 This PR is included in @superdoc-dev/mcp v0.3.0-next.37 The release is available on GitHub release |
|
🎉 This PR is included in vscode-ext v2.3.0-next.81 |
|
🎉 This PR is included in @superdoc-dev/react v1.2.0-next.79 The release is available on GitHub release |
|
🎉 This PR is included in superdoc-cli v0.8.0-next.55 The release is available on GitHub release |
|
🎉 This PR is included in superdoc v1.30.0-next.38 The release is available on GitHub release |
|
🎉 This PR is included in superdoc-sdk v1.8.0-next.40 |
|
🎉 This PR is included in superdoc-cli v0.9.0 The release is available on GitHub release |
|
🎉 This PR is included in superdoc v1.32.0 The release is available on GitHub release |
|
🎉 This PR is included in @superdoc-dev/mcp v0.4.0 The release is available on GitHub release |
|
🎉 This PR is included in @superdoc-dev/react v1.3.0 The release is available on GitHub release |
|
🎉 This PR is included in vscode-ext v2.4.0 |
|
🎉 This PR is included in superdoc-sdk v1.9.0 |
3 participants
Removes 28 hand-written
.d.tsfiles that lived next to.tssource with no generator producing them. They had drifted: 14 of 39 sibling pairs disagreed with their source, including the example block instyle-engine/src/index.d.tsshowingimport { resolveStyle }from a function that does not exist. TypeScript prefers a sibling.d.tsover the inferred.ts, so consumers were reading fictional APIs and would have shipped imports that returnedundefinedat runtime.tsc -band the affected per-package builds (@superdoc/contracts,@superdoc/style-engine,@superdoc/pm-adapter,@superdoc/layout-engine,@superdoc/super-editor) all pass without the shadows; types are now inferred from the source.A new gate (
scripts/check-dts-shadows.mjs) refuses any sibling.ts+.d.tspair underpackages/. Wired into lefthook pre-commit andci-superdoc.ymlso the regression is caught both locally and on PRs.packages/collaboration-yjs/is allowlisted in the gate. That package builds withtsup --dts, and its src.d.tsfiles are load-bearing compilation leaves; deleting them breaks the build until tsup is reconfigured. The allowlist entry documents this and SD-2922 stays open for the follow-up.Related: SD-2828 (published TypeScript surface contract, complete - this is the internal-package follow-up that ticket explicitly punted).
Verified:
pnpm run type-checkclean; per-package builds clean; gate exits 0 today and exits 1 with file list when a shadow is reintroduced.