Migrate CKEditor 5 to TypeScript - follow-ups

[arkflpc]

This issue serves as an umbrella for all the tasks that need to be done after migrating the project to TypeScript, including bug fixes, code refactoring, and feature enhancements.

Additionally, this issue is also a platform for the community to share their feedback and suggestions regarding the TypeScript migration.

Tasks

• [TS] Update README #13831
• [TS] Suggest and validate names of plugins and commands at type level #13743
• [TS] Revise code style guide to cover TypeScript #11723
• [TS] Allow running tests written in TS (automated, manual) #11888
• [TS] Add linter rules for enforcing imports of module augmentation and no explicit return type #13434
• [TS] Add types to DLLs #13855
• [TS] Remove community types in @types #13830
• [TS] Add editingDowncast and dataDowncast in conversion.for #13750
• Move EditorUI to ckeditor5-ui #12853
• [TS] Research how to make import and re-export cleaner #12720
• [TS] Remove I type parameter from Collection #12763
• [TS] Merge the "number" type into PriorityString #12218

Community Feedback

We encourage the community to share their feedback and suggestions regarding the TypeScript migration. If you encounter any bugs or issues related to the migration, please report them here. We value your feedback and will use it to improve the quality of the project. Thank you for your support and contribution.

[mmichaelis]

Replace "export type" by "export" in index.ts

We are just at upgrading to CKEditor 37.x, thankfully being able to incorporate your TypeScript typings!

We stumbled across some "export type", though, in index.ts, where we require a full export. From discussion, our perception is, that index.ts is the "declared public API" and direct imports like from imageinline should be prevented – but are the only workaround for us now.

We would require the following to be full exports instead:

ckeditor5/packages/ckeditor5-image/src/index.ts

Line 37 in 629f8ab

export type { default as ImageInline } from './imageinline';

ckeditor5/packages/ckeditor5-image/src/index.ts

Line 39 in 629f8ab

export type { default as ImageUtils } from './imageutils';

ckeditor5/packages/ckeditor5-image/src/index.ts

Line 40 in 629f8ab

export type { default as ImageBlockEditing } from './image/imageblockediting';

We may provide details, why we need these. For ImageInline, for example, we do not support ImageBlock, so that we directly need to reference ImageInline instead of the aggregator Image.

Add to index.ts

And the following, we would like to be exported in the corresponding index.ts:

ckeditor5/packages/ckeditor5-core/src/commandcollection.ts

Line 16 in 629f8ab

export default class CommandCollection implements Iterable<[ string, Command ]> {

ckeditor5/packages/ckeditor5-link/src/ui/linkactionsview.ts

Line 27 in 629f8ab

export default class LinkActionsView extends View {

ckeditor5/packages/ckeditor5-link/src/ui/linkformview.ts

Line 42 in 629f8ab

export default class LinkFormView extends View {

ckeditor5/packages/ckeditor5-editor-classic/src/classiceditoruiview.ts

Line 20 in 629f8ab

export default class ClassicEditorUIView extends BoxedEditorUIView {

Same as above, we may provide details, why we require them, so that we may discuss alternative paths we could take.

For Reference: Our Usages

We added a comment to our upgrade PR to reference issues reported here: CoreMedia/ckeditor-plugins#145 (e.g., CoreMedia/ckeditor-plugins@55200cb).

Similar to:

// See ckeditor/ckeditor5#12027.

So, you may see, for example, that we only require CommandCollection on integration test level. Thus, it may be completely valid to state, that it should not go into index.ts just for this purpose.

[Witoso]

@mmichaelis we appreciate the feedback! We will review it here: #13864

[mmichaelis]

Just created another issue in this context:

• Typings to be optimized for Autosave (save requires DataApi/DataApiMixin) / editor.getData on the Editor type #13904

[Inviz]

Cant find a way to import ViewTreeWalker from engine. Having to use ReturnType<ViewPosition['getWalker']> for now

[Inviz]

Also please add inferred types for event listeners. It's backwards now, type has to be provided and that validates the event name. Currently, i can not import types like ViewDocumentCopyEvent for example to listen to event.

[arkflpc]

Hi @Inviz ,

I've created a ticket to provide ViewTreeWalker (#14158).

Regarding the events: You can import ViewDocumentCopyEvent from ckeditor5-clipboard. I.e.

import { ViewDocumentCopyEvent } from '@ckeditor/ckeditor5-clipboard'

We tried to provide automatic type inference for event listeners, like lib.dom.d.ts do. However, it turned out to be very difficult with the event system we have in ckeditor5. The option to refactor events was out-of-scope, because almost everything relies on this. If you are interested in solutions we tried, please read this comment.

[CKEditorBot]

There has been no activity on this issue for the past year. We've marked it as stale and will close it in 30 days. We understand it may still be relevant, so if you're interested in the solution, leave a comment or reaction under this issue.

[CKEditorBot]

We've closed your issue due to inactivity. We understand that the issue may still be relevant. If so, feel free to open a new one (and link this issue to it).