fix(types): chain commands return ChainableCommandObject, not boolean (SD-2334)#2773
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5617cd149f
ℹ️ 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".
… (SD-2334) ChainableCommandObject used KnownChainedCommands (empty for npm consumers since module augmentation doesn't survive the package boundary) plus a Record<string, ...> index signature that conflicted with run: () => boolean. This caused TypeScript to infer intermediate chain methods could return boolean, breaking chains like chain().setTextSelection(...).setMark(...).run(). Apply the same fix used for EditorCommands: compose AllChainedCommands from direct imports of each command interface, transformed via Chainified<T> to return ChainableCommandObject. Remove the Record<string, ...> fallback. Same treatment for CanObject to prevent can().chain() type conflicts.
…upport Address review feedback: - Extract shared AllCommandSignatures base type used by EditorCommands, ChainableCommandObject, and CanObject (eliminates triple-maintained 10-interface intersection) - Add AugmentedChainedCommands/AugmentedCanCommands so consumers who extend ExtensionCommandMap via module augmentation still get their custom commands on chain() and can()
c2ebff1 to
2e5ff6d
Compare
|
🎉 This PR is included in @superdoc-dev/react v1.0.0-next.37 The release is available on GitHub release |
|
🎉 This PR is included in vscode-ext v1.1.0-next.83 |
|
🎉 This PR is included in template-builder v1.3.0-next.43 The release is available on GitHub release |
|
🎉 This PR is included in esign v2.2.0-next.41 The release is available on GitHub release |
|
🎉 This PR is included in superdoc v1.24.0-next.80 The release is available on GitHub release |
|
🎉 This PR is included in superdoc-cli v0.5.0-next.81 The release is available on GitHub release |
|
🎉 This PR is included in superdoc-sdk v1.3.0-next.82 |
|
🎉 This PR is included in superdoc-sdk v1.4.0 |
|
🎉 This PR is included in superdoc v1.25.0 The release is available on GitHub release |
|
🎉 This PR is included in superdoc-cli v0.6.0 The release is available on GitHub release |
|
🎉 This PR is included in vscode-ext v2.3.0-next.1 |
|
🎉 This PR is included in template-builder v1.5.0-next.1 The release is available on GitHub release |
|
🎉 This PR is included in esign v2.3.0-next.1 The release is available on GitHub release |
PR #2565 (SD-2261) replaced the handwritten
index.d.tswith auto-generated types fromChainedCommands.ts, fixing theChainableCommandObject | booleanreturn type union that IT-344 and IT-780 reported. However, the replacement type still usedKnownChainedCommands(empty for npm consumers — module augmentation doesn't survive the package boundary) and aRecord<string, ...>index signature fallback, which left two problems:noPropertyAccessFromIndexSignatureincompatibility — consumers with this TS strict flag can't use dot access on chain methods (chain.setTextSelection(...)errors with "must be accessed with bracket notation")unknown[]sinceKnownChainedCommandsis emptyThis PR applies the same fix already used for
EditorCommands: composeAllChainedCommandsfrom direct imports of each command interface, transformed viaChainified<T>to returnChainableCommandObject. Same treatment forCanObject.EditorCommandsRecord<string, ...>index signature — works with all TS strict flagsrun()is unambiguously() => booleancan().chain()returnsChainableCommandObject, not an overloaded intersectionResolves SD-2334, IT-344, IT-780