refactor(superdoc): migrate SuperDoc.js to TypeScript#3479
Merged
Conversation
Renames the main SuperDoc public class to .ts and adopts TS-native
syntax for the surface (`declare` fields, real type imports,
explicit return types, EventMap generic).
What changed
- packages/superdoc/src/core/SuperDoc.{js,ts}: 2402-line rename.
Added `import type { ... }` for every type previously declared as
JSDoc `@typedef {import(...)}`. Replaced the `@type {Config}`
on `this.config` with `config: InternalConfig` (the post-#init
shape; defeats the literal-narrowing TS infers from the default
object). Added 15 `declare` class fields for late-init runtime
state (`activeEditor`, `toolbar`, `isCollaborative`,
`socket`, etc.) so TS knows the shape without emitting an own
property initializer - matches the previous runtime exactly.
Marked the four internal handles (`superdocStore`,
`commentsStore`, `highContrastModeStore`, `commentsList`,
`app`) as `private declare` so the consumer-typecheck fixture
still asserts they are not part of the public TS surface.
Annotated ~60 method/parameter signatures (Editor, User,
RuntimeDocument, string, etc.) using the existing JSDoc as the
source of truth. Added explicit return types on `state`,
`getPresentationEditorForDocument`, `navigateTo`,
`scrollToElement`, and `openSurface` so deep-type-audit does
not see new `any` leaks. Converted the JSDoc `SuperDocEventMap`
typedef to a real TS `interface` and applied it as
`EventEmitter<SuperDocEventMap>` so unknown event names are a
compile error (consumer-typecheck fixture asserts this).
Converted ~9 `/** @type {InternalConfig} */ (this.config)` JSDoc
casts to plain `this.config` reads (no longer needed) and the
remaining few JSDoc casts to TS `as` casts.
- packages/superdoc/src/core/types/index.ts: added three runtime-
only members to `RuntimeDocument` (`rulers?`, `restoreComments?`,
`removeComments?`). RuntimeDocument is internal-only (not re-
exported from the public facade), so this does not change the
consumer surface; it removes the need to cast at call sites.
What did not change
- No `as any` shortcuts. The one `any` adapter
(`asEventListener`) preserves the pre-existing JSDoc behavior
and carries an eslint-disable + documenting comment.
- No new blanket suppressions. The two existing `@ts-expect-error`
lines (`__APP_VERSION__` const-replace + `?url` asset import)
are preserved.
- No `// @ts-check` line in the .ts file.
- packages/superdoc/dist/superdoc/src/public/index.d.ts and
index.d.cts are byte-identical to main.
- `packages/layout-engine/pm-adapter/src/*.d.ts` build artifacts
are NOT included (intermittent leak from the build step).
Verified
- pnpm check:types -> PASS (0 errors; baseline after rename was 241).
- pnpm check:public:superdoc -> PASS, 9 stages, 158s.
- deep-type-audit-supported-root PASS (no new `any` regressions
on the supported root surface).
- pnpm --filter superdoc test -> 1054 / 1054 tests pass.
- Public-facing `dist/.../public/index.d.{ts,cts}` byte-identical
to main. SuperDoc.d.ts (internal, relocated by ensure-types)
changes shape (new EventMap interface, TS-native imports) but
does not surface `any` on the supported root per the audit.
…fter migration
Pre-PR cleanup pass on the SuperDoc.ts migration. The TS-native
annotations from the prior commit made several JSDoc constructs
redundant; this strips them and refreshes stale prose. No runtime
change, no public surface change.
- SuperDoc.ts: removed the dead `@typedef {import(...)}` block (15
declarations now shadowed by real `import type { ... }`), the
large `@typedef {{ ... }} SuperDocEventMap` JSDoc and its 11
payload `@typedef` siblings (replaced by the TS interface block
added earlier), and `@param`/`@returns`/`@type` tags whose
type info now duplicates the TS signature. Kept the prose comments
that explain *why* (exception union shape, fonts-resolved
listener-transport, runtime cast rationale for asEventListener).
Updated the one in-body comment that still said "once SuperDoc.js
is brought under the SD-2863 checkJs gate" - that's done.
- core/types/index.ts: three comment refreshes ("internal
SuperDoc.js callsites" → "internal SuperDoc callsites"). Import
specifiers stay as `./SuperDoc.js` - they resolve to the .ts
source via customConditions and renaming the extension would
ripple to every importer.
Footprint: 2,586 → 2,386 lines in SuperDoc.ts (-200), 128 → 62 JSDoc
tags. Public-facing `dist/.../public/index.d.{ts,cts}` still
byte-identical to main.
Verified:
- pnpm check:types → PASS
- pnpm check:public:superdoc → PASS, 9 stages, 152.5s
- pnpm --filter superdoc test → 1054 / 1054 tests pass
…ource comment Second cleanup pass surfaced by review: - SuperDoc.ts: removed the remaining JSDoc-style casts that became dead text in a .ts file. Two patterns: - `/** @type {InternalConfig} */ this.config` (8 occurrences) - the field is already `config: InternalConfig`, so the prefix was just stray text TS ignores. - `/** @type {X} */ (value)` inline local-var casts - TS infers from `value` directly. Also removed standalone `/** @type {X} */` lines preceding field/var declarations where TS inference matches the JSDoc intent. Kept JSDoc on public/static API where it carries prose. - public/index.ts: refreshed the `// Source: ./core/SuperDoc.js` comment to point at the .ts source (the import specifier stays `.js` because it resolves through customConditions + the package exports map). After this pass: SuperDoc.ts is 2,374 lines (was 2,386); remaining JSDoc tags carry prose, not redundant types. Verified: - pnpm check:types → PASS - pnpm check:public:superdoc → PASS, 9 stages, 160.3s - pnpm --filter superdoc test → 1054 / 1054 tests pass - public dist .d.ts byte-identical to main
Lint surfaced two import-x/no-duplicates warnings (two import-type blocks both from './types/index.js') and one no-unused-vars warning (Document imported but only used in JSDoc, never as a TS annotation). Merged the two ./types blocks, dropped Document, alphabetized both type-import lists for stable diffs.
Both warnings are intentional patterns surfaced (not introduced) by the migration. Adding scoped eslint-disable comments + explanations: - asEventListener: widened the rule disable from next-line to enable/disable block so it covers the cast on the return line too. The any here is the documented runtime contract. - syncCleanup placeholder: added a comment + eslint-disable-next-line for the intentional no-op that gets reassigned to the real cleanup once the sync observer registers.
The previous comment leaned on customConditions + the package exports map. For this relative import, the actual rule is TS-side .js→.ts specifier resolution. Reworded to say what is true.
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: 6ad8dc77cc
ℹ️ 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".
CI caught: the VS Code extension's contract test statically reads the SuperDoc source via readFileSync to assert the public API shape, and the path was hardcoded to SuperDoc.js. After the .js → .ts rename in this PR, the open() call fails with ENOENT. Updated the path to SuperDoc.ts. Kept the read direct (no js → ts fallback): the source is now TS, period. Verified: pnpm --filter superdoc-vscode-ext test → 14 / 14 pass.
Codex review caught a real migration regression: SuperDoc.search was
annotated as `search(text: string)` during the .js → .ts migration,
narrower than both the JSDoc on the line above (`@param {string |
RegExp}`) and the runtime contract (search.js:485-510 explicitly
branches on `isRegExp(patternInput)` and threads regex through to
`new RegExp(...)`).
Net effect on consumers: `superdoc.search(/foo/i)` was valid TS on
main and became a TS error after migration, even though the runtime
still accepts it.
Fix is one line: `search(text: string | RegExp)`. Added a fixture
regression guard in tests/consumer-typecheck/src/search-match.ts:
a runtime call site `sd.search(/hello/i)` plus an AssertEqual on
`Parameters<SuperDoc['search']>[0]` so the next migration can't
re-narrow without CI flagging it.
Verified: pnpm check:types → PASS; pnpm check:public:superdoc →
PASS, 9 stages, 155s.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
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.
SuperDoc's main public class is now TypeScript. The runtime is unchanged; the migration moves load-bearing contracts to TS-native syntax so future refactors get real class-shape feedback. Six commits: the migration itself, then five cleanup commits surfaced by review (dead JSDoc removal, dead cast removal, duplicate-import consolidation, lint-warning fixes, source-comment refresh).
What changed
packages/superdoc/src/core/SuperDoc.{js,ts}(2,379 lines). Realimport type { ... }for the load-bearing types (alphabetized, deduplicated).config: InternalConfig = { ... }annotation defeats the literal-shape narrowing TS infers from the default object (the dominant error class on rename, ~140 of the initial 241). 15declareclass fields for late-init runtime state (activeEditor,toolbar,isCollaborative,socket, etc.) so TS knows the shape without emitting an own-property initializer - matches the previous runtime exactly. The five internal handles (superdocStore,commentsStore,highContrastModeStore,commentsList,app) areprivate declareso they stay off the public TS surface (consumer-typecheck fixture asserts this). ~60 method/parameter signatures annotated using existing JSDoc as the source of truth. Explicit return types added onstate,getPresentationEditorForDocument,navigateTo,scrollToElement, andopenSurfacesodeep-type-audit-supported-rootsees no newanyleaks.SuperDocEventMapconverted from JSDoc typedef to TSinterfaceand applied asEventEmitter<SuperDocEventMap>so unknown event names are a compile error.Cleanup commits: removed the dead JSDoc typedef/event-map blocks now shadowed by TS interfaces/imports, dropped the
/** @type {InternalConfig} */ this.configJSDoc-style casts (the field is already typed, the prefix was just stray text TS ignores), removed standalone/** @type {X} */lines preceding declarations whose TS type is now explicit or trivially inferred. Some JSDoc tags remain where they still serve documentation purposes (public API docstrings,@deprecatedmarkers,@exampleblocks, multi-line prose); they don't duplicate type information that TS now carries.packages/superdoc/src/core/types/index.ts: added three runtime-only members toRuntimeDocument(rulers?,restoreComments?,removeComments?). RuntimeDocument is not re-exported by the public facade; public facade output verified byte-identical to main.packages/superdoc/src/public/index.ts: refreshed the// Source: ./core/SuperDoc.jscomment to point at the.tssource. The.jsimport specifier is intentional for ESM output and resolves to the.tssource during TypeScript builds.What did NOT change
as anyshortcuts. The oneanyadapter (asEventListener) preserves the pre-existing JSDoc behavior and is wrapped in a scopedeslint-disable @typescript-eslint/no-explicit-anyblock with a documenting comment explaining why the looseness is correct (EventEmitter dispatches whatever each emit site supplies).@ts-expect-errorlines (__APP_VERSION__const-replace +?urlasset import) are preserved.// @ts-checkline in the.tsfile.packages/superdoc/dist/superdoc/src/public/index.d.tsandindex.d.ctsare byte-identical to main. Diff verified viamd5sumagainst baseline captured from main before rename.Verified
pnpm check:types→ PASS. 0 errors. (Baseline after rename was 241; reduced to 0 via the changes above, no shortcuts.)pnpm check:public:superdoc→ PASS, 9 stages, 160.3s. Including:consumer-typecheck-matrix(validates published .d.ts across bundler/node16/nodenext × strict × skipLibCheck).deep-type-audit-supported-root(no newanyregressions on supported root).superdoc-stores-private,superdoc-events,whiteboard-data-shape,search-match) pass.pnpm --filter superdoc test→ 1054/1054 tests pass.pnpm --filter superdoc run lint→ 0 warnings on SuperDoc.ts..d.tsfiles byte-identical to main. SuperDoc.d.ts (internal, relocated by ensure-types) changes shape (new EventMap interface block, TS-native type imports) but does not surfaceanyon the supported root per the audit.Review focus: the
private declarechoices on the five internal handles and thedeclare(no-initializer) choices on the 15 late-init runtime fields. Both follow the discipline of "TS shape only, no runtime change." If any of these should actually be public, flag the specific field. TheSuperDocEventMapinterface is a direct translation of the prior JSDoc typedef - payload references unchanged.