Skip to content

[OP-18993] Add TypeScript tooling (1/3) - #135

Open
myabc wants to merge 4 commits into
masterfrom
code-maintenance/OP-18993-typescript-tooling
Open

myabc wants to merge 4 commits into
masterfrom
code-maintenance/OP-18993-typescript-tooling

Conversation

@myabc

@myabc myabc commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Ticket

https://community.openproject.org/wp/OP-18993

What are you trying to accomplish?

First of three PRs that convert this repository to strict TypeScript. This one adds the tooling and converts no source file, so the bundle is byte-identical to master.

After this PR a file opts in to type checking by being renamed from .js to .ts. .ts files are checked in strict mode; .js files stay importable and unchecked.

Unlike #109 and #110, the conversion lands in reviewable steps: tooling here, then non-plugin code, then plugins and entry files.

What approach did you choose and why?

  • tsconfig.json: replaces the existing file, which was unusable (DOM lib only, no include). Strict, noEmit, allowJs without checkJs, target: ES2022 so class fields keep emitting natively, and verbatimModuleSyntax so an import is only dropped when written as import type.
  • src/types.d.ts: declares window.I18n and window.OpenProject, and the string modules raw-loader produces for *.svg.
  • webpack: ts-loader with transpileOnly. Bundling strips types; checking stays in npm run typecheck.
  • jest: @babel/preset-typescript, configured to match tsc (onlyRemoveTypeImports, allowDeclareFields).
  • eslint: typescript-eslint for .ts files. Bans any, @ts-ignore and @ts-nocheck; @ts-expect-error needs a description. Rules that would force runtime edits (prefer-const and similar) are off, because the conversion PRs change types only.
  • CI: runs npm run typecheck.

Version pins, each forced by a peer: typescript@~5.9 (typescript-eslint supports <6.1, npm latest is 7.x), @babel/preset-typescript@^7 (repo is on Babel 7), @types/jest@^29 (repo is on Jest 29).

Verification

Gate Result
npm run typecheck pass
npm test 20 suites, 256 passed, 7 skipped (same as master)
npm run lint pass
npm run build pass
Bundle vs master ckeditor.js byte-identical (cmp, sha256 fb9a98c31f92406e… on both)

Each new gate was also checked to fail when it should, using a temporary canary file: a type error fails typecheck; @ts-ignore and any fail lint; a .ts module imports and runs under jest and bundles under webpack.

AI involvement

Directed: written by Claude Code from a design agreed with @myabc, then reviewed adversarially by Codex (no findings).

@myabc
myabc added this pull request to stack #138 October 7, 2026 18:40
@myabc
myabc marked this pull request as ready for review October 7, 2026 18:44
Copilot AI balanced review requested due to automatic review settings October 7, 2026 18:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No confirmed blocking issues remain in this tooling-only change; source conversion is deferred to subsequent PRs.

0 open findings

What changed in this PR

Adds tooling for the staged TypeScript migration of the CommonMark CKEditor build, without converting runtime source files.

Changes:

  • Enables strict TypeScript checking and declares browser globals and SVG imports.
  • Adds TypeScript support to webpack, Jest, and ESLint.
  • Adds dependencies, documentation, and a CI typecheck step.
File Description
webpack.config.js Resolves and transpiles TypeScript modules.
tsconfig.json Enables strict checking while leaving JavaScript unchecked.
src/​types.d.ts Declares OpenProject globals and SVG modules.
README.md Documents the typechecking workflow.
package.json Adds tooling dependencies and typecheck command.
package-lock.json Locks updated tooling dependencies.
jest.config.js Enables TypeScript test transformation.
eslint.config.mjs Adds TypeScript lint rules.
babel.config.js Configures TypeScript transpilation for tests.
.github/​workflows/​ci.yml Runs typechecking in CI.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

myabc added 4 commits October 7, 2026 21:14
Replaces the unusable tsconfig (DOM lib only, no include) with a strict
one and adds a typecheck script. JavaScript stays importable but
unchecked, so a file opts in to checking by being renamed to .ts.

Declares the window globals that OpenProject core provides and the
string modules that raw-loader produces for SVG imports.

TypeScript is pinned to 5.9 because typescript-eslint does not support
newer majors yet.

https://community.openproject.org/wp/OP-18993
Adds ts-loader in transpile-only mode, so bundling strips types without
checking them; checking stays in the typecheck script. Jest gets the
Babel TypeScript preset, configured to match tsc: imports survive unless
written as type imports, and declare fields emit nothing.

No source file is converted yet, so the bundle is unchanged.

https://community.openproject.org/wp/OP-18993
Adds typescript-eslint for .ts files under src and tests. It bans
explicit any, @ts-ignore and @ts-nocheck, and requires a description on
@ts-expect-error, so every escape from the type system carries a reason.

Rules that would demand edits to runtime code (prefer-const and the
like) are off for now: the conversion changes types only.

https://community.openproject.org/wp/OP-18993
@myabc
myabc force-pushed the code-maintenance/OP-18993-typescript-tooling branch from d226c99 to b1543ca Compare October 7, 2026 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants