fix(superdoc): tighten Modules.toolbar typedef (SD-2867 phase A)#3051
fix(superdoc): tighten Modules.toolbar typedef (SD-2867 phase A)#3051caio-pizzol merged 3 commits intomainfrom
Conversation
Probing checkJs against SuperDoc.js found nine reads on
config.modules.toolbar (selector, excludeItems, groups, icons, texts,
fonts, hideButtons, responsiveToContainer) all collapsing to type
'Object' for consumers. The runtime accepts each field but the public
typedef lists only `[toolbar] {Object}`.
Promotes Modules.toolbar to a structured shape covering the full set
of fields the implementation reads. Each field is the inverse of an
existing top-level Config.toolbar* alias (toolbarGroups, toolbarIcons,
toolbarTexts) so consumers that already pass these via the modules
form get IDE help.
Verified locally: enabling // @ts-check on SuperDoc.js drops 7 of the
24 'type Object' errors. The remaining 17 sit on internal function
signatures inside SuperDoc.js (e.g. private method @param {Object})
and are picked up by SD-2867 phase B.
superdoc-bot
Bot
added
review: thorough
review: quick
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: fc8e1baf2e
ℹ️ 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".
…e A) Review pass found three runtime mismatches in the previous commit and two wording problems: - selector was typed `string | HTMLElement`, but findElementBySelector in super-editor calls `.startsWith()` and `document.querySelector` — HTMLElement crashes. The public docs also say "must be a string selector, not a DOM element reference." Narrowed to `string`. - groups was typed `string[]` only, but super-toolbar.js reads `Object.keys(this.config.groups)` and `Object.values(...).flatMap(...)` — the runtime accepts an object map, used in the canonical `examples/features/custom-toolbar` and documented as the supported form. Widened to `string[] | Record<string, string[]>`. Also fixed the example values: defaults are `'left' | 'center' | 'right'`, not `'edit' | 'format' | 'insert'`. - customButtons was missing entirely. It is documented in packages/superdoc/AGENTS.md and demoed in examples/features/custom-toolbar; closing the typedef without it would TS-reject a documented public field. Added with a structural Array<Record<string, unknown>> placeholder; a future ticket can promote ToolbarItem to the public surface and tighten this. Also softened the parent-level alias claim: only selector, groups, icons, and texts have Config.toolbar* aliases, not all eight fields.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
… phase A)
Second review pass found two more runtime mismatches in the previous
commit:
- The string[] arm of `groups: string[] | Record<string, string[]>`
is not actually supported. SuperDoc.js spreads `...moduleConfig` into
SuperToolbar's config, and SuperToolbar's #makeToolbarItems calls
`Object.values(this.config.groups).flatMap(...)` on the array — for
an array of group ids that flattens to character codes and silently
hides every toolbar item. Group-id arrays belong on the top-level
`Config.toolbarGroups` alias, not here. Narrowed to
`Record<string, string[]>` only and updated the doc to call out the
alias for the array form.
- `fonts` was typed `Array<{ name, value }>`, but FontConfig (the type
super-toolbar already documents and consumes) is `{ key, label, ... }`.
Switched to `import('@superdoc/super-editor').FontConfig[]` so the
typedef matches the public type that's already exported.
The HTMLElement-selector and customButtons fixes from the previous
commit stand.
|
🎉 This PR is included in @superdoc-dev/mcp v0.3.0-next.29 The release is available on GitHub release |
|
🎉 This PR is included in @superdoc-dev/react v1.2.0-next.72 The release is available on GitHub release |
|
🎉 This PR is included in vscode-ext v2.3.0-next.74 |
|
🎉 This PR is included in superdoc v1.30.0-next.31 The release is available on GitHub release |
|
🎉 This PR is included in superdoc-cli v0.8.0-next.47 The release is available on GitHub release |
|
🎉 This PR is included in superdoc-sdk v1.8.0-next.33 |
|
🎉 This PR is included in superdoc-cli v0.8.0 The release is available on GitHub release |
|
🎉 This PR is included in superdoc-sdk v1.8.0 |
|
🎉 This PR is included in @superdoc-dev/mcp v0.3.0 The release is available on GitHub release |
|
🎉 This PR is included in superdoc v1.31.0 The release is available on GitHub release |
2 participants
The SuperDoc.js probe under SD-2867 found nine reads on `config.modules.toolbar.*` (`selector`, `excludeItems`, `groups`, `icons`, `texts`, `fonts`, `hideButtons`, `responsiveToContainer`) that all collapse to TypeScript `Object` for consumers. The runtime accepts each field, but the public typedef lists only `[toolbar] {Object}`. Customers using `modules: { toolbar: { ... } }` get no IDE help and strict-mode TypeScript rejects valid configs.
Promotes `Modules.toolbar` to a structured shape covering the full set of fields the implementation reads. Each new field is the inverse of an existing top-level `Config.toolbar*` alias (`toolbarGroups`, `toolbarIcons`, `toolbarTexts`) — keeping both forms is intentional, since the modules path is the preferred one for new code.
This is the typedef-side slice of SD-2867. Enabling `// @ts-check` on SuperDoc.js still surfaces ~134 errors against internal function signatures (private `@param {Object}` declarations, implicit-any params, strict-null) — those are picked up by phase B.
Verified: temporarily enabling `// @ts-check` on SuperDoc.js locally drops 7 of the 24 `type 'Object'` errors at the public-contract callsites. `pnpm --filter superdoc run check:jsdoc` still passes (no enrollment change).