Repository navigation
Conversation
myabc
added this pull request to stack #138
October 7, 2026 18:40
myabc
marked this pull request as ready for review
October 7, 2026 18:43
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Several new declarations misstate core contracts and suppress unsafe CKEditor node handling.
1 open finding
What changed in this PR
Converts the remaining plugins, tests, and entry points to strict TypeScript while removing JavaScript support.
Changes:
- Adds strict typings for plugins and OpenProject integration APIs.
- Migrates entry points and tests from JavaScript to TypeScript.
- Updates Webpack, ESLint, and TypeScript configuration.
| File | Description |
|---|---|
webpack.config.js |
Uses the TypeScript entry point. |
tsconfig.json |
Removes JavaScript support. |
tests/plugins/op-resizer-guard.test.ts |
Types resizer guard tests. |
tests/plugins/code-block.test.ts |
Types code-block test fixtures. |
tests/helpers/button-disabler.test.ts |
Types editor test stubs. |
src/types.d.ts |
Adds editor globals and dialog events. |
src/plugins/op-upload-resource-adapter.ts |
Types the upload adapter. |
src/plugins/op-upload-plugin.ts |
Types adapter registration. |
src/plugins/op-source-code.plugin.ts |
Types the source preview callback. |
src/plugins/op-resizer-guard/op-resizer-guard-plugin.ts |
Converts the resizer guard plugin. |
src/plugins/op-macro-wp-quickinfo/predicate.ts |
Types the quick-info predicate. |
src/plugins/op-macro-wp-quickinfo/op-macro-wp-quickinfo-plugin.ts |
Types quick-info conversions. |
src/plugins/op-macro-wp-button/utils.ts |
Types work-package widget helpers. |
src/plugins/op-macro-wp-button/op-macro-wp-button-toolbar.ts |
Types toolbar attributes. |
src/plugins/op-macro-wp-button/op-macro-wp-button-plugin.ts |
Converts the plugin entry. |
src/plugins/op-macro-wp-button/op-macro-wp-button-editing.ts |
Types editing conversions. |
src/plugins/op-macro-wiki-page-link/op-macro-wiki-page-link-plugin.ts |
Types wiki-link conversions. |
src/plugins/op-macro-wiki-page-link/op-macro-wiki-page-link-create-new.ts |
Types new-page dialog handling. |
src/plugins/op-macro-wiki-page-link/op-macro-wiki-page-link-add-existing.ts |
Types existing-page dialog handling. |
src/plugins/op-macro-wiki-page-link/macro-url.ts |
Adds the typed URL helper. |
src/plugins/op-macro-wiki-page-link/macro-url.js |
Removes the JavaScript helper. |
src/plugins/op-macro-wiki-page-link/insert-wiki-page-link.ts |
Types wiki-link insertion. |
src/plugins/op-macro-toc-plugin.ts |
Types ToC rendering helpers. |
src/plugins/op-macro-list-plugin.ts |
Types removed-plugin inspection. |
src/plugins/op-macro-embedded-table/utils.ts |
Types embedded-table helpers. |
src/plugins/op-macro-embedded-table/embedded-table-toolbar.ts |
Types toolbar services. |
src/plugins/op-macro-embedded-table/embedded-table-plugin.ts |
Converts the plugin entry. |
src/plugins/op-macro-embedded-table/embedded-table-editing.ts |
Types embedded-table editing. |
src/plugins/op-macro-child-pages/utils.ts |
Types child-page widget helpers. |
src/plugins/op-macro-child-pages/op-macro-child-pages-toolbar.ts |
Types toolbar attributes. |
src/plugins/op-macro-child-pages/op-macro-child-pages-plugin.ts |
Converts the plugin entry. |
src/plugins/op-macro-child-pages/op-macro-child-pages-editing.ts |
Types child-page editing. |
src/plugins/op-image-attachment-lookup/op-image-attachment-lookup-plugin.ts |
Types attachment converters. |
src/plugins/op-help-link-plugin/op-help-link-plugin.ts |
Converts the help-link plugin. |
src/plugins/op-custom-css-classes-plugin.ts |
Types custom-class converters. |
src/plugins/op-content-revisions/utils.ts |
Types revision utilities. |
src/plugins/op-content-revisions/ui.ts |
Types revision dropdown UI. |
src/plugins/op-content-revisions/storage.ts |
Defines revision storage records. |
src/plugins/op-content-revisions/op-content-revisions.ts |
Types the revisions plugin. |
src/plugins/op-content-revisions/command.ts |
Types revision command arguments. |
src/plugins/op-attachment-listener-plugin.ts |
Types attachment-removal handling. |
src/plugins/code-block/widget.ts |
Types code-block widget helpers. |
src/plugins/code-block/converters.ts |
Types code-block converters. |
src/plugins/code-block/code-block.ts |
Converts the plugin entry. |
src/plugins/code-block/code-block-toolbar.ts |
Types toolbar attributes. |
src/plugins/code-block/code-block-editing.ts |
Types editing integration. |
src/plugins/code-block/click-observer.ts |
Types double-click observation. |
src/op-types.ts |
Defines OpenProject integration contracts. |
src/op-plugins.ts |
Converts the plugin registry. |
src/op-config-customizer.ts |
Types configuration customization. |
src/op-ckeditor.ts |
Converts editor entry classes. |
src/op-ckeditor-config.ts |
Types the default editor configuration. |
src/helpers/button-disabler.ts |
Types toolbar disabling helpers. |
README.md |
Documents strict TypeScript checking. |
eslint.config.mjs |
Removes obsolete JavaScript lint rules. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+79
to
+81
| timezone: { | ||
| formattedRelativeDateTime(timestamp: number): string; | ||
| }; |
Pure rename, including its test; types follow in the next commit. https://community.openproject.org/wp/OP-18993
Adds the macros service to the OpenProject service types, with the code block editor it opens. Runtime code is unchanged. https://community.openproject.org/wp/OP-18993
Pure rename; types follow in the next commit. https://community.openproject.org/wp/OP-18993
Types the work package button, embedded table, child pages, wiki page link, quickinfo, table of contents and macro list plugins, and adds the core services they call to the service types. Declares the dialog:close document event that core fires. Runtime code is unchanged. Typing surfaced four things, left as they are and marked TODO: - The work package button and child pages widgets pass an object where the widget label should be a string. - The embedded table widget reads a label that is never assigned. - The macro list reads pluginName from removePlugins entries, which is undefined for entries given as strings. https://community.openproject.org/wp/OP-18993
Pure rename, including the resizer guard test; types follow in the next commit. https://community.openproject.org/wp/OP-18993
Types content revisions, custom CSS classes, image attachment lookup, uploads, the attachment listener, the source toggle and the resizer guard test, and adds the core services they call to the service types. Runtime code is unchanged. The button disabler now takes CKEditor's Editor: its UI view declares the toolbar, so only the private item list needs a cast. Typing surfaced these, left as they are and marked TODO: - Revisions UI checks `items.count`, which arrays do not have. - Custom CSS classes passes a priority to add(), which ignores it. - The upload adapter resolves with undefined when a request fails. - Image attachment lookup throws when the editor has no resource. https://community.openproject.org/wp/OP-18993
Pure rename; types and the webpack entry follow in the next commit. https://community.openproject.org/wp/OP-18993
Types the plugin list, default configuration, configuration customizer and editor classes, and points webpack at the renamed entry. The editor classes declare createCustomized with `declare static`, so the statics are still assigned after the class definitions, in the same order as before. The window exports that core reads are declared on Window. Our widget toolbar keys are registered on EditorConfig. https://community.openproject.org/wp/OP-18993
No .js file is left under src or tests, so allowJs goes and the lint blocks for JavaScript sources and tests go with it. Jest keeps transforming .js because dependencies are transformed too. https://community.openproject.org/wp/OP-18993
myabc
force-pushed
the
code-maintenance/OP-18993-typescript-plugins
branch
from
October 7, 2026 19:14
a115444 to
99f6aab
Compare
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
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.
Ticket
https://community.openproject.org/wp/OP-18993
What are you trying to accomplish?
Third of three PRs that convert this repository to strict TypeScript. It converts all plugins and the entry files, then stops accepting JavaScript. After it, no
.jsfile is left undersrcortests. Stacked on the non-plugin PR.The conversion changes types only. The production bundle built from this branch is byte-identical to the one built from
master.Declaration output for core (the other goal of the work package) is not part of this PR and follows separately.
What approach did you choose and why?
Each area is two commits: a pure rename, then the types. The last two commits type the entry files and remove
allowJs.createCustomizedwithdeclare static. The statics are still assigned after the class definitions, in the same order.OPConstrainedEditor,OPClassicEditor,OPEditorWatchdog) are declared onWindow.src/op-types.tsgains the core services the plugins call: macros, external query configuration, Turbo requests, notifications, timezone and attachments.EditorConfig.dialog:closedocument event core fires is declared onDocumentEventMap.What typing found
Left as they are and marked
TODO(OP-18993), because this PR must not change behaviour. Each deserves its own fix:OpUploadResourceAdapter#upload()catches a failed request and resolves withundefined, whereUploadAdapterrequires a response or a rejection. Codex reproduced the consequence: CKEditor then dereferences the missing response and throws, without removing the failed image. This is the most important of the list.record.items.count, which arrays do not have, so the "no revisions" branch only runs when there is no record at all.{priority: 'high'}toadd(), which takes one argument and ignores it.this.label, which is never assigned.pluginNamefromremovePluginsentries, which isundefinedfor entries given as strings.openProjectconfiguration.TimezoneService#formattedRelativeDateTime(), which core declares as taking a datetime string.<code>.Escape hatches in the finished code base
@ts-expect-erroradd()priority above)ascastsunknown!)anyMost casts narrow
getAttribute()results, which CKEditor types asunknown, or DOM nodes whose kind a filter already guarantees. Casts that hide a mismatch carry a comment saying why.Verification
npm run typecheckallowJsremovednpm testmaster)npm run lintnpm run buildmasterckeditor.jsandckeditor.cssbyte-identical (cmp).jspredecessor.jsundersrcandtestsNot done: a manual smoke test in core. Given the byte-identical bundle it should show nothing, but it has not been run.
AI involvement
Directed: written by Claude Code from a design agreed with @myabc, then reviewed adversarially by Codex. Codex found no runtime change and flagged the upload defect listed above.