fix(superdoc): hide internal Pinia stores from SuperDoc public type surface (SD-3213f)#3416
Merged
Merged
Conversation
…urface (SD-3213f) Drains 715 supported-root any findings (950 to 235) by hiding the three internal Pinia stores from SuperDoc's emitted .d.ts. Type-surface hide, not runtime privacy: the fields still exist on the runtime instance and internal callers across the package keep working. Consumers can no longer reach into them via TypeScript. Required a host-contract refactor: marking the stores @Private alone broke createHeadlessToolbar({ superdoc }) because HeadlessToolbarSuperdocHost.superdocStore was public optional and TS does not let a private field satisfy a public optional. The new fixture in this PR caught the regression before merge. - SuperDoc.js: @Private on superdocStore, commentsStore, highContrastModeStore. Two new narrow public methods replacing the raw-store reach: getPresentationEditorForDocument(documentId) and getComment(commentId). Return types intentionally wide (PresentationEditor | null and Record<string, unknown> | null) so the public surface does not pull Pinia type graph. - HeadlessToolbarSuperdocHost: superdocStore? removed; two optional narrow methods added matching the SuperDoc additions. - resolveToolbarSources: prefers superdoc.getPresentationEditorForDocument when present; legacy superdocStore.documents[] reach kept as fallback for custom host stubs that pre-date the narrow method. - track-changes.ts: prefers superdoc.getComment when present; legacy commentsStore.getComment reach kept as fallback. - New consumer fixture (src/superdoc-stores-private.ts) pins both: - Negative: superdoc.superdocStore/commentsStore/highContrastModeStore are @ts-expect-error. - Positive: createHeadlessToolbar({ superdoc }) and createSuperDocUI({ superdoc }) compile with a real SuperDoc instance. - supported-root allowlist regenerated: 950 to 235 entries (75% drop). Cleanup of the remaining 'as never' casts in create-super-doc-ui.ts internals is tracked separately as SD-3213g (not blocking the drain). Verified: - pnpm run build: exit 0. - typecheck-matrix.mjs: 59/59 (2 new SD-3213f scenarios added). - deep-type-audit.mjs --strict-supported-root: PASS (235 in, 0 new, 0 stale). - super-editor unit tests: 13104 passed. - SuperDoc top offender count in audit: 832 to 117.
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: 0514867b6d
ℹ️ 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".
This comment was marked as resolved.
This comment was marked as resolved.
…cated union branch (SD-3213f) Codex bot caught a backward-compat regression in the parent commit (0514867). Removing superdocStore? from HeadlessToolbarSuperdocHost broke inline custom host stubs that pass createHeadlessToolbar({ superdoc: { ..., superdocStore: { documents: [...] } } }); at TS-strict via excess-property checks. The runtime fallback in resolveToolbarSources still accepts the legacy shape, so the host TYPE needed to advertise it too. Without this fix, typed consumers upgrading hit a forced 'any' cast. Fix: HeadlessToolbarSuperdocHost becomes a union of two branches that share a common base: - HeadlessToolbarSuperdocHostNarrow: SD-3213f narrow shape with getPresentationEditorForDocument / getComment. SuperDoc satisfies this branch directly. - HeadlessToolbarSuperdocHostLegacy: pre-SD-3213f shape with typed superdocStore.documents[]. Marked @deprecated. Kept so inline custom host stubs that pre-date the narrow methods keep compiling. commentsStore was intentionally NOT added to the legacy branch: it was never on the exported host type pre-SD-3213f. The track-changes runtime fallback uses a wide Record<string, any> param, not the host type. Adding it now would be public-surface growth, not backward-compat. Fixture: src/superdoc-stores-private.ts gains a positive assertion that an inline legacy-shape host compiles via the deprecated union branch. The fixture I added in the parent commit covered SuperDoc and narrow-method hosts but missed the legacy inline case; this regression class is now locked. Verified: - pnpm run build: exit 0. - typecheck-matrix.mjs: 59/59. - deep-type-audit.mjs --strict-supported-root: PASS (235 in, 0 new, 0 stale).
…itorForDocument, getComment) Codecov flagged the two new methods on SuperDoc.js as uncovered. The consumer fixture from the parent commit (0514867) proves the TypeScript contract but does not execute them. Existing tests cover the underlying commentsStore.getComment indirectly through canPerformPermission, but not the new narrow public wrapper. Adds 8 focused Vitest cases under 'SD-3213f narrow host methods': - getPresentationEditorForDocument: - returns the presentation editor for the matching documentId - returns null when no document matches the id - returns null when the matched document has no presentation editor - returns null for empty or non-string documentId - getComment: - delegates to commentsStore.getComment and returns the result - returns null when commentsStore returns no comment for the id - returns null when commentsStore.getComment is missing - returns null for empty or non-string commentId These pin the public replacement for the raw-store reach we hid in 0514867, so future PRs cannot regress the contract silently. Verified: 1030 / 1030 superdoc unit tests pass.
The parent commits added a narrow-host dispatch in two bridge sites: - resolve-toolbar-sources.ts prefers superdoc.getPresentationEditorForDocument over the legacy superdocStore.documents[] reach. - track-changes.ts (via toolbar-registry) prefers superdoc.getComment over commentsStore.getComment. Legacy paths already had test coverage (resolve-toolbar-sources.test.ts for superdocStore, toolbar-registry.test.ts for commentsStore). The narrow paths and the narrow-vs-legacy precedence were not pinned, so a future refactor could silently drop the narrow branch and only the legacy code path would run. Adds 3 focused tests: - resolve-toolbar-sources.test.ts: narrow path works in isolation (getPresentationEditorForDocument returns the editor when the host provides only the narrow method). - resolve-toolbar-sources.test.ts: precedence (when both narrow and legacy are present, narrow wins; legacy is not invoked). - toolbar-registry.test.ts: precedence for the tracked-change enricher (when both superdoc.getComment and commentsStore.getComment are present, narrow wins and the permission helper receives the narrow-method comment). The precedence tests implicitly cover narrow-alone behavior (if narrow wins when both are present, it must work when only narrow is present), so no separate narrow-alone test for getComment. Verified: 75 / 75 in the two affected files pass.
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.
Drains the single biggest source of public `any` reach in SuperDoc: the internal Pinia stores (`superdocStore`, `commentsStore`, `highContrastModeStore`) that were leaking through `SuperDoc.d.ts` and collapsing consumer IntelliSense at every depth that reached them.
Type-surface hide, not runtime privacy. The fields still exist on the runtime instance. Internal callers across the package keep working. Consumers can no longer reach into them through TypeScript.
Required a host-contract refactor: `@private` alone broke `createHeadlessToolbar({ superdoc })` because `HeadlessToolbarSuperdocHost.superdocStore?` was public optional and TS does not let a private field satisfy a public optional. The new fixture caught the regression before merge.
Cleanup of the remaining `as never` casts in `create-super-doc-ui.ts` is tracked separately as SD-3213g; not blocking the drain.
Drain delta: total findings 1817 to 1102 (-715). Top SuperDoc offender 832 to 117.
Verified: build exit 0, matrix 59/59, audit PASS, super-editor unit tests 13104 passed.