fix(types): drain Tier 4 any from public contract (SD-2834)#3258
Merged
Conversation
…ProofingError (SD-2834) Three surgical fixes to the curated public-contract file `packages/superdoc/src/core/types/index.ts`. Each one was visible in the deep audit's tier-4-public-contract bucket and represents direct customer IntelliSense pain on a documented public API. 1. `EditorTransactionEvent.transaction: any` -> `Transaction` from `prosemirror-state`. Anyone hovering over an `onTransaction` callback's transaction param now sees the real ProseMirror type with `.docChanged`, `.steps`, `.selection`, etc., instead of `any`. `prosemirror-state` is already an optional peer dep, consistent with the existing pattern for `@hocuspocus/provider`. 2. `Config.onAwarenessUpdate.states: any[]` -> `AwarenessState[]`. New small public type defines the documented fields SuperDoc populates per remote client (`user`, `clientId`, `color`) plus an index signature for application-specific keys. y-protocols' Awareness type itself uses `Record<string, any>` for the value side, so a precise imported type was not available; defined locally rather than collapsing to `unknown[]` per the audit README's guidance against draining with `unknown` when a real shape is documentable. Removes the existing `eslint-disable @typescript-eslint/no-explicit-any` on the same line. 3. `ProofingError.cause?: any` -> `unknown`. Error causes are genuinely opaque (whatever the proofing provider threw), so `unknown` is the correct level of precision per the Error-cause convention. Consumers narrow with `instanceof` or shape checks before reading. Also re-exports `AwarenessState` via the JSDoc typedef in `packages/superdoc/src/index.js`, adds the AssertNotAny assertion to `tests/consumer-typecheck/src/all-public-types.ts`, and updates the existing modules-config-passthrough fixture to consume the new type through documented field access. Verified: - node tests/consumer-typecheck/typecheck-matrix.mjs: 53/0 - node tests/consumer-typecheck/deep-type-audit.mjs: 1799 -> 1791 (tier-4-public-contract: 26 -> 18) - AssertNotAny<AwarenessState> passes (check-public-types gate)
superdoc-bot
Bot
added
review: thorough
review: careful
and removed
review: thorough
labels
![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: 784db7fddd
ℹ️ 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".
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Code review caught that `packages/react/src/types.ts:71` defines a
hand-mirrored `SuperDocTransactionEvent` interface that still had
`transaction: any` after the canonical `EditorTransactionEvent` was
narrowed to `Transaction` in the previous commit. The mirror's JSDoc
explicitly says "Mirrors superdoc's EditorTransactionEvent", so React
consumers using `@superdoc-dev/react` would have kept seeing
`transaction: any` and lost IntelliSense - the exact symptom this PR is
meant to fix.
Sync the React mirror:
- Add `Transaction` to the `import type { ... } from 'superdoc'`
(avoids adding `prosemirror-state` as a new dep; superdoc already
re-exports the type via JSDoc typedef)
- Change `transaction: any` -> `transaction: Transaction`
- Drop "or transaction-like payload" from the JSDoc to match the
canonical wording
Verified the awareness and proofing fixes don't need React mirrors -
those flow through `Config` directly (which React extracts from
SuperDoc's constructor parameters) so consumers already see them.
Verified:
- pnpm --filter @superdoc-dev/react run type-check: passes
Code review caught that the previous AwarenessState shape pointed
consumers at fields that don't exist at runtime. The
`awarenessStatesToArray` helper at `shared/common/collaboration/awareness.ts`
returns `{ clientId, ...value.user, color }` per entry - i.e., the
remote user's `name`, `email`, `image` are SPREAD onto the top of the
state, not nested under `state.user`.
The first cut typed `user?: User`, which made `state.user?.name`
typecheck (always undefined at runtime) while real fields like
`state.name` and `state.email` only resolved through the
`unknown` index signature. That's worse than `any` - it documents
the wrong shape.
Flip AwarenessState to `extends User`, matching the runtime spread.
Update the doc comment to spell out the flatten and point consumers at
`state.name` / `state.email` directly. Update the consumer fixture to
read the flattened fields (`state.name`, `state.email`, `state.clientId`,
`state.color`, plus the index-signature `state['customField']`).
Verified:
- pnpm --filter superdoc run pack:es: ok
- node tests/consumer-typecheck/typecheck-matrix.mjs: 53/0
- node tests/consumer-typecheck/deep-type-audit.mjs: 1791 (unchanged)
Contributor
|
🎉 This PR is included in @superdoc-dev/mcp v0.3.0-next.92 The release is available on GitHub release |
Contributor
|
🎉 This PR is included in vscode-ext v2.3.0-next.138 |
Contributor
|
🎉 This PR is included in @superdoc-dev/react v1.2.0-next.136 The release is available on GitHub release |
Contributor
|
🎉 This PR is included in superdoc-cli v0.8.0-next.107 The release is available on GitHub release |
Contributor
|
🎉 This PR is included in superdoc v1.30.0-next.88 The release is available on GitHub release |
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
2 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.
Three surgical fixes to the curated public-contract file
packages/superdoc/src/core/types/index.ts. Each one was visible in the deep audit'stier-4-public-contractbucket and represents direct IntelliSense pain on a documented public API.EditorTransactionEvent.transaction: any→Transactionfromprosemirror-state. Hovering over theonTransactioncallback's transaction param now shows the real ProseMirror type (.docChanged,.steps,.selection, etc.).prosemirror-stateis already an optional peer, matching the existing pattern for@hocuspocus/provider.Config.onAwarenessUpdate.states: any[]→AwarenessState[]. New small public type covering the documented fields SuperDoc populates per remote client (user,clientId,color).y-protocols/awarenessitself types states asMap<number, { [x: string]: any }>so a precise imported type wasn't available; defined locally rather than collapsing tounknown[].ProofingError.cause?: any→unknown. Removes theeslint-disable @typescript-eslint/no-explicit-anythat was sitting on the awareness line.AwarenessStateis intentionally permissive: the[key: string]: unknownindex signature is there so consumers using a custom provider can still attach application-specific fields. The documented fields give IDE help; the index signature keeps the pass-through contract.cause?: unknownis intentional, not a lazyanyreplacement. Error causes are genuinely opaque (whatever the proofing provider threw) and consumers should narrow withinstanceofor shape checks before reading. This is the Error-cause convention TC39 adopted.Modules.config-passthrough fixture updated to consume the new type through documented field access (
state.user?.name,state.clientId,state.color) plus an index-signature read (state['customField']) to assert the pass-through still works.Compatibility note. Consumers writing custom callback signatures with
any[]will see a strict-mode error after this lands. The index signature keeps the surface permissive for application-specific fields; updates should be 1-2 lines per callsite.Verified:
node tests/consumer-typecheck/typecheck-matrix.mjs: 53 passed, 0 failednode tests/consumer-typecheck/deep-type-audit.mjs: 1799 → 1791 findings (tier-4-public-contract: 26 → 18)AssertNotAny<AwarenessState>passes (check-public-types gate)Linear: SD-2834