fix(#7410): add proper typings for registerWidget and CmsBackendClass - #7411
Open
fgnass wants to merge 34 commits into
Open
fix(#7410): add proper typings for registerWidget and CmsBackendClass #7411fgnass wants to merge 34 commits into
fgnass wants to merge 34 commits into
Conversation
This reverts commit cbdb932.
Member
|
That is a big diff to review 😅 We are working on some other features at the moment, I really hope to get to your PRs soon! |
Contributor
Author
|
I know 😅 No worries, I'm happily using my fork in the meantime. I squashed the commits of the parent PR to at least reduce the number of commits in this one and make it possible to review both in sequence. Let me know if there is anything else I can do to make your life easier. |
Resolved conflicts: - package-lock.json: deleted, main migrated the repo to pnpm. - package.json / pnpm-workspace.yaml: took main's pnpm+catalog form, kept the PR's `tsc --build` type-check and tsbuildinfo cleanup, bumped the catalog's typescript to ^5.3.0 and added @types/semaphore. Kept main's type-check step in test:ci. - tsconfig.json: kept the PR's composite/project-references layout, adopted main's automatic JSX runtime (jsx: react-jsx, jsxImportSource: @emotion/react). - decap-cms-core/package.json: main's catalog deps, minus the decap-cms-editor-component-image peer dep the PR inverted. - decap-cms-editor-component-image: kept the PR's typed index.tsx but dropped the React import, which main no longer needs; decap-cms-core is a workspace:* devDependency. - decap-cms-backend-test/src/implementation.ts: took main's moveFile refactor. - widget-markdown slate.spec.js: kept the PR's relocated test-helpers path plus main's jsxRuntime/eslint pragmas.
Merging main brought two things the PR's declaration emit had never met:
pnpm's isolated node_modules and immutable 4. `tsc --build` and
`nx run-many -t build` both failed with 9 errors; each is fixed at the source
rather than by hoisting node_modules.
Undeclared dependencies that only resolved via npm's flat tree:
- decap-cms-core imports `Pluggable` from `unified`
- decap-cms-backend-gitlab and decap-cms-backend-github reach `graphql` and
`apollo-link` types through apollo-client
- decap-cms-widget-markdown's serializers infer `@types/unist` nodes
Inferred types that TS cannot name under pnpm (TS2742) now carry explicit
annotations, which is also what makes the emitted .d.ts portable — they
reference `graphql`, `apollo-link` and `unist` by package name instead of a
`.pnpm/...` realpath:
- gitlab queries: `DocumentNode`
- GraphQLAPI.mutate: `Promise<FetchResult<OperationVariables>>`
- markdown serializers: `@returns {import('unist').Node}`
- DecapCmsApp: `CMS`
Two type errors were real:
- backend-test moveFile, added by main, passes the now-optional
`RepoFile.content` to writeFile, same as the other call site.
- createEditorComponent claims to return EditorComponentOptions, but converts
`fields` to Immutable and consumers read it that way (`fields.update(...)`,
`field.get('widget')`). immutable 3 typed `fromJS` as `any` and hid this;
immutable 4 does not. Named that shape `RegisteredEditorComponent` and used
it for the registry and `getEditorComponents()` instead of casting the lie
away.
✅ Deploy Preview for decap-cms ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
The PR built the machinery to generate declarations from source but stopped
short in two ways: it relaxed the checks that would have exercised that
machinery, and it kept a hand-written interface as the gatekeeper, so the
published API surface could still omit APIs that exist.
Strictness, restored to what main has today:
- decap-cms-core had noImplicitAny, strictNullChecks and strictFunctionTypes
turned off, hiding 47 errors. All three are back. Most were unannotated
parameters in the newly converted registry.ts; the rest were real:
- actions/config.ts declared `let collectionI18n: CmsI18nConfig` and then
assigned undefined to it (same for fileI18n)
- actions/entries.ts leaked `undefined` out of createEmptyDraftData through
an over-broad reduce annotation; the reducer only ever returns the
DraftEntryData accumulator, so it is now typed as such (the large diff is
prettier reindenting the body, not a logic change)
- registerWidget could store a widget with no control component; it now
reports that the same way the object branch already did
- getWidgets ran a `produce` that never mutated its draft, purely to have
somewhere to put an `any`; plain map does the same thing
- .eslintrc.js disabled @typescript-eslint/no-explicit-any repo-wide with a
"TODO enable in future". It covered 10 violations in 3 files, all added by
this PR. Rule restored, violations removed, zero left.
Types that did not match the implementation:
- GetAssetFunction described a one-argument function returning a plain object.
The real one, boundGetAsset in actions/media.ts, takes (path, field) and
returns an AssetProxy. Every real call site was a type error, which is why
editor-component-image annotated its own parameters as `any` — the reference
component opting out of the types it was demonstrating. Type fixed, `any`s
gone, and that component is now the acceptance test for it.
- CmsEventListener.handler declared one parameter; invokeEvent passes the
registration options as a second.
- PreviewStyleOptions.raw was required even though the options argument is not.
- CmsRegistry.mediaLibraries did not include the options registerMediaLibrary
stores alongside each library.
- toPreview's `fields` is an Immutable List, and is absent when a component is
previewed outside the editor.
Making the generated typings authoritative:
- `DecapCmsCore` now uses `satisfies CMS` instead of a `: CMS` annotation. An
annotation only checks the implementation against the interface; it cannot
catch the interface being too narrow. That is how getWidgets,
getEventListeners, removeEventListener, invokeEvent, getCustomFormats,
getCustomFormatsExtensions and getCustomFormatsFormatters — seven APIs that
have always existed — were invisible to TypeScript users on main and stayed
invisible under this PR. They resolve now, and CMS lists them too.
- scripts/types-fixture type-checks the built declarations the way an external
consumer would, and runs in CI after build:demo. Verified it fails when an
API is dropped from the public export.
- scripts/test-types-coverage.mjs asserts every published package is wired into
the declaration build, so a new package cannot silently fall out of
`tsc --build` the way four already had.
Coverage and cleanup:
- decap-cms-backend-forgejo, decap-cms-ui-auth, decap-cms-widget-richtext and
decap-cms-widget-uuid post-date the PR and were never added to it, so they
emitted no types and, once the root tsconfig moved to `include: []` plus
references, were not type-checked at all. All four are wired in now.
- Deleted three stale hand-written index.d.ts files. core's was resurrected by
an earlier merge and duplicated src/types; lib-auth's was already dead;
ui-auth's imported from `decap-cms-lib-util/src`, which that package does not
publish, so it was broken for consumers. decap-cms-app's `files` still listed
an index.d.ts the PR had deleted.
- `pnpm develop` was broken in every package that gained build:types: `--watch`
landed on `run-p`, which rejects it. Present before the merge; no CI job runs
develop, so nothing caught it.
- Moved widget-markdown's test helper back out of src/. The PR moved it in but
left widget-richtext's identical copy alone, and under src/ it shipped to npm
in dist/ with a generated h.d.ts.
martinjagodic
added a commit
that referenced
this pull request
Sep 9, 2026
Cypress compiles cypress.config.ts with ts-node, which runs in CommonJS and so forces `module: CommonJS`. The root tsconfig sets `moduleResolution: nodenext`, and since TypeScript 5.0 that pair is rejected outright: error TS5110: Option 'module' must be set to 'NodeNext' when option 'moduleResolution' is set to 'NodeNext'. Cypress reports it as "Your configFile is invalid" and every e2e shard fails before running a single test. It does not reproduce on main because main is still on TypeScript 4.9, which did not enforce the constraint; bumping the catalog to ^5.3.0 for `satisfies` surfaced it. Give ts-node a matching pair of its own, scoped to ts-node only so nothing else changes. Same fix as fgnass's in #7411. Co-Authored-By: Felix Gnass <fgnass@gmail.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
3 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.
This is a follow-up PR that supersedes #7402 and fixes #7410.
It also addresses the
// TODO: type properlycomment that was previously in index.d.tsIn general, this shows how we can gradually migrate the codebase to TypeScript.