ckeditor / ckeditor5

Powerful rich text editor framework with a modular architecture, modern integrations, and features like collaborative editing.
https://ckeditor.com/ckeditor-5
Other
9.59k stars 3.7k forks source link

Migrate CKEditor 5 to TypeScript #11704

Closed Reinmar closed 1 year ago

Reinmar commented 2 years ago

Back in 2015 when we were bootstrapping CKEditor 5 TypeScript wasn't yet mature enough and didn't yet seem as a safe choice taken the long horizon of CKEditor 5's life and fate of e.g. CoffeeScript or Backbone (that were the popular choices at that time).

Nowadays, for a large scale, complex project such as CKEditor 5, TypeScript is a clear choice. We've been actually looking at it for a longer time but it was clear that we need to pick the right moment for a migration. The moment has come 🥳

The main goals of the migration:

We're right now cooking a plan of action, so I won't share more details for now. Some two last points that I want to mention for now are:

Keep your fingers crossed and stay tuned for more information.

arkflpc commented 2 years ago
  • Once again I'd like to thank @fedemp and everyone else who contributed to the community-driven DefinitelyTyped. For at least a couple of months it's still the place to go when you need typings for CKEditor 5.

I don't know if it's possible for us to stay compatible with that, though. Should we?

fedemp commented 2 years ago
  • Once again I'd like to thank @fedemp and everyone else who contributed to the community-driven DefinitelyTyped. For at least a couple of months it's still the place to go when you need typings for CKEditor 5.

I don't know if it's possible for us to stay compatible with that, though. Should we?

Official typings do not need to be compatible with DT typings. And if official typings somehow contradict those at DT, the official should be the definitive answer (assuming CKEditor is going to be rewritten in TS).

fedemp commented 2 years ago

This project will take several months due to the scale of CKEditor 5. We'll be migrating step by step, most likely starting from the infrastructure and one or two first packages.

You team surely know better, but I suggest you start with utils, core, and lastly engine. utils have very little dependencies, and engine is a huge monster to tackle.

Reinmar commented 2 years ago

Hi @fedemp! I tried to reach out to you on LinkedIn. Could you check your invitations or write back to me at p.koszulinski@cksource.com?

arkflpc commented 2 years ago

Some ideas for post-MVP:

arkflpc commented 2 years ago

Status Update:

What we've done:

  1. We rewrote the ckeditor5-utils package to TypeScript (#11755).
  2. We're now able to run both manual and automated tests (#11888).
  3. We have working rules for eslint (#11719).
  4. We can build DLLs (almost done, #11718).

It's all on a feature branch yet. But as soon as we'll finish updating the release process (#11720, work-in-progress), we can get to master branch and release it. The packages on NPM won't contain typings yet, because that would conflict with the community-provided ones. We're going to start generating them after we'll finish the work on ckeditor5-engine (#11724, work-in-progress), ckeditor5-ui (#11726) and ckeditor5-core (#11727) and stabilize the typings.

fedemp commented 2 years ago

The packages on NPM won't contain typings yet, because that would conflict with the community-provided ones.

When the time comes, open an issue on DefinitelyTyped to remove those. :+1:

arkflpc commented 2 years ago

We will, @fedemp. We're going to stabilize them first.

Reinmar commented 2 years ago

It's worth mentioning that CKEditor 5 v35.0.0 is out and it's the first version where TS code was used (namely @ckeditor/ckeditor5-utils) 👏

As for what's next...

We're right now reviewing https://github.com/ckeditor/ckeditor5/pull/12188 and engine was by far the biggest and most complex package to port.

Next packages to migrate: core and utils. Soon, we'll be able to start porting features which should be easy to do concurrently and much much faster.

arkflpc commented 2 years ago

Status update

We released ckeditor5-engine (#11724) in TypeScript. ckeditor5-ui (#11726) and ckeditor5-core (#11727) are done on feature branch (ck/11726-11727-ts-core-ui). They will be merged in few next weeks.

The next milestone is to select API documentation generator and integrate it with our tooling. After it's done, we will go on with other packages that are in JavaScript yet.

Reinmar commented 1 year ago

Some status update.

We're doing good progress migrating packages. We covered more than 50% of them, including all the core ones (that are by far the biggest ones). So in general, it feels like we're 75% done.

Once all packages are migrated, we'll start publishing the types to npm. So far, those are omitted as they would be incomplete.

Keep your fingers crossed, TS is coming :runner:

arkflpc commented 1 year ago

Some stats about progress:

Language files blank comment code
TypeScript 600 17589 62313 54875
JavaScript 199 4898 14206 16609
-------- -------- -------- -------- --------
SUM: 799 22487 76519 71484
colindawson commented 1 year ago

I'm working on a new application and am in the process of deciding what editor I'd like to base the application on. It's not liklely to be going live for at least 6 months (loads to do, maybe longer depending on what's else I need to implement) My preferred choice, especially as it's a very active project is to use CKEditor, but as you can guess, I'm working in Typescript with React. Is there some way a work around that I can use (noobie instructions would be appreciated) that will allow me to use CKEditor5 in my application whilst waiting for this major refactor?

I feel that the alternative I'm looking at would be shooting myself in the foot in the long run.

arkflpc commented 1 year ago

@colindawson,

CKEditor is usable in TypeScript projects today. There are excellent community provided types you can rely on.

The refactor you mentioned is only about adding types to the code.

When we are ready to provide our types in npm packages, they will not be compatible. But this should affect mostly custom plugins that are interacting with core components of the editor. The types for API used for embedding and configuring CKEditor will not be modified much. Even in this case, only types will have to be adjusted.

colindawson commented 1 year ago

@arkflpc I'm extremely confused about this, it's all new to me. Looking at the link you posted, it's gave me an idea to look for @types/ckeditor and I found this https://www.npmjs.com/package/@types/ckeditor__ckeditor5-core Is this the kind on thing that you are referring too?

I've figured out enough to make it work, there's nothing in the link you provided, what I needed to do is create a file called types/ckeditor.d.ts with the following content

declare module '@ckeditor/ckeditor5-build-classic' {
    import ClassicEditor from '@ckeditor/ckeditor5-build-classic';

    const ClassicEditor = typeof ClassicEditor;

    export {ClassicEditor};
}

declare module '@ckeditor/ckeditor5-react' {
    import ClassicEditor from '@ckeditor/ckeditor5-build-classic';
    import Event from '@ckeditor/ckeditor5-utils/src/eventinfo'
    //import { EditorConfig } from '@ckeditor/ckeditor5-core/src/editor/editorconfig'
    import * as React from 'react';
    const CKEditor: React.FunctionComponent<{
        disabled?: boolean;
        editor: typeof ClassicEditor;
        data?: string;
        id?: string;
        config?: EditorConfig;
        onReady?: (editor: ClassicEditor) => void;
        onChange?: (event: Event, editor: ClassicEditor) => void;
        onBlur?: (event: Event, editor: ClassicEditor) => void;
        onFocus?: (event: Event, editor: ClassicEditor) => void;
        onError?: (event: Event, editor: ClassicEditor) => void;
    }>
    export { CKEditor };
    }

This was enough to get my react app from failing to compile and show the editor. I'm sure this is a huge hack. And I'm looking forward to being able to scrap this when typescript support is ready. I much prefer this to using a different control. I've posted this here, so that any other newbies to typescript and CKEditor5 can be helped whilst waiting for the Typescript rewrite to be completed.

mmichaelis commented 1 year ago

@colindawson, we just finished more than a year of development to replace CKEditor 4 with CKEditor 5 (find the sources at https://github.com/CoreMedia/ckeditor-plugins). And yes, the package you spotted is one of them. Another (for the classic editor) is: https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/ckeditor__ckeditor5-editor-classic.

We started off with custom typings, which is a very error-prone and lengthy process, as if you pull one thread, you will get all the rest (so, hats off to CKEditor-team addressing this eventually!).

When your build is set up and ready, there is actually nothing more to do, than:

A minimal package with CKEditor 5 dependency and some tooling around ckeditor5-core can be found here, which may be one reference: https://github.com/CoreMedia/ckeditor-plugins/tree/main/packages/ckeditor5-core-common

If you experience any issues with the typings, we recommend adding:

// @ts-expect-error - Typing issue at DefinitelyTyped

or similar. This is because the typings are not always in line with the CKEditor 5 sources and/or documentation. But overall, they are quite complete and a very good start. (@ts-expect-error is better than @ts-ignore as it will inform you, once the typings got updated accordingly and may help to migrate to official typings by CKEditor later)

And just regarding your remark:

My preferred choice, especially as it's a very active project is to use CKEditor

Stick to it! In our experience, it has a very well-designed architecture and I think we really stretched it to its limits.

Inviz commented 1 year ago

Really bummed out by current situation that old types dont seem to work with TS 4.9+, while the new types are not published yet (even partially). We're stuck with both older version of TS and older version of editor. Even incomplete types are better than nothing

colindawson commented 1 year ago

Really bummed out by current situation that old types dont seem to work with TS 4.9+, while the new types are not published yet (even partially). We're stuck with both older version of TS and older version of editor. Even incomplete types are better than nothing

WIth 199 files to do, don't hold you breath that it's going to get done faster. As a new user to CKEditor, I was bummed out that combined with my limited knowledge of typescript, it didn't work out of the box. As you can see above, asking a couple of questions and doing a little digging, I've managed to get CKEditor 5 working on my Typescript React app. Having watched the project for a couple of days. I'm really excited to be using this component, and would much prefer that the implementation is done right I'll deal with the technical debt of using my hack when the time comes, but right now. I believe that the team working on this are doing a great job and I know that with a bit of patience it will be resolved "soon"(tm)

Inviz commented 1 year ago

Which version of typescript do you use? I'm using the CKEditor5 with the types for about 8 months by now. The types module really really helped to get my project off the ground (we dont even use ckeditor5's own ui, it's a total overhaul), but the types package is outdated and as i mentioned does not work for 4.9/5 for us because of default exports thing, that i could not personally resolve. If you use 4.9, let me know what settings do you use and what are the import calls you do for importing individual files. I can not update TS on our project because of this.

I understand that partial types are not released currently, because it would conflict with the old types package? So it's a curse and a blessing to have the old types.

jacquesg commented 1 year ago

I could also not get @types/ckeditor__ckeditor5-core to work, had to resort to creating a file very similar to the one shared by @colindawson.

Sadly, no simple works-out-of-the-box solution here :(

colindawson commented 1 year ago

Just to be clear, whilst what I did worked. I need to use more advanced things from CKEditor, so for now, I've decided to change my editor over to using jsx files, instead of tsx. This way, I'm avoiding typescript for the moment, with the intent to come back and refactor once the typescript version is released.

Reinmar commented 1 year ago

We're about to finish the code migration. We should be done with both the open source and commercial code by the end of February.

The code release to npm is blocked, though, by the rest of our tooling (e.g. release scripts) that need to catch up. It's now planned for the beginning of April.

But, perhaps we could share all the typings even now without breaking anything? Like, publishing them in a temporary npm package like @ckeditor/ckeditor5-experimental-typings. Could those be used easily then? Having them out a month before an official release would help validating some bits of them.

Inviz commented 1 year ago

That would be really grand if we could have typings earlier than that. I'd really appreciate it

balean12 commented 1 year ago

Hello, what is the status of CKEditor 5 typings? Is the final release out yet? Thank you

Reinmar commented 1 year ago

We've just (like an hour ago :D) published 37.0.0-alpha.0 of all open source packages of CKEditor 5. 

All these packages in this specific version contains typings for the entire code available in them :tada:

That's an early alpha version and naturally some things may still change. For instance, we're dealing now with TypeScript issues with generating .d.ts files for module augmentation. Unfortunately, publishing types for such a large ecosystem with so many weird requirements is not a trivial task.

Note: We did not publish 37.0.0-alpha.0 for the commercial packages. Nor plan to publish typings for them before publishing a stable 37.0.0 version (planned for April).

Inviz commented 1 year ago

I'll be damned, it's actually working. Amazing job! I'm finding some inconsistencies, but they are minor.

1) Breaking: selectable in model writer is set to Node type, but according to docs it's https://ckeditor.com/docs/ckeditor5/latest/api/module_engine_model_selection-Selectable.html this. So it doesnt accept position at this time. 2) Breaking: Plugin doesnt have .set method (for observable props)... it used to work 3) EventInfo signature seems to have been simplified from EventInfo<doc, string> to EventInfo<string> 4) I could not make assignment to Editor.builtinPlugins to work:

Type 'typeof import("/Users/sitecore/Sites/feaas-components/ckeditor5/node_modules/@ckeditor/ckeditor5-link/src/autolink")' is not assignable to type 'PluginConstructor<Editor>' 5) I dont use config often, but i'm getting error on assigning heading. I think this is what Reinmar meant talking about module augmentation?

Inviz commented 1 year ago

@Reinmar thank you CKEditor team!

Inviz commented 1 year ago

@Reinmar can we update import paths, so the types will include .js in paths? It is backward compatible, but this way it'll be compatible with other module resolution strategies (e.g. nodenext). At the moment i'm still stuck at 4.8 because of it. Tried everything i could.

filipsobol commented 1 year ago

@Inviz We will look into the issues you listed in your first message and get back to you soon either with an update or an explanation.

Can you elaborate on your request to include .js in paths? Why is this necessary, and what problems are you experiencing with the current approach?

Inviz commented 1 year ago

@filipsobol Thanks Filip!

The issue is that node16+ module setting requries .js extensions, otherwise it can't find files. The symptom is that types can't be found for the package. It says "type X is not exported by ckeditor5". It's necessary for modern esm modules projects

https://github.com/microsoft/TypeScript/issues/49083 https://stackoverflow.com/questions/65873101/node-requires-file-extension-for-import-statement/65874173#65874173

This is because node.js follows the ES6 standard which forbids the interpreter from guessing filename extensions. You will also need to add file extensions if you want to use import in plain javascript in browsers.

filipsobol commented 1 year ago

@Inviz Could you please set up a small reproduction that we can use? In alpha.2 we will make some (rather small) changes to how code from packages is imported and it would help us greatly.

Inviz commented 1 year ago

@Inviz Could you please set up a small reproduction that we can use? In alpha.2 we will make some (rather small) changes to how code from packages is imported and it would help us greatly.

I'm embarrased to say but i think i can not reproduce anymore what I tried to achieve with it. The only thing that is reliably not working is importing types from files directly like:

import type { default as ModelElement } from '@ckeditor/ckeditor5-engine/src/model/element.js'

However doing it like:

import type { Element as ModelElement } from '@ckeditor/ckeditor5-engine'

works. I can't figure out in which configuration did using .js extensions helped it. I'll try to use the second method of importing types (from root of the package) and see how far i go. If i stumble upon a problem, i'll update my reproduction repo (i took link away for now, as it's not super useful)

filipsobol commented 1 year ago

Importing directly from the root of the package is exactly the change we want to introduce in alpha.2 (mentioned in my previous message). If you find that we don't export something you need, please let us know.

Inviz commented 1 year ago
Inviz commented 1 year ago

Otherwise I managed to make it work on 4.9. I import now all types from root of packages, instead of .js files like before. Thanks!

filipsobol commented 1 year ago

I'm glad to hear this and want to thank you for the feedback - it reassures us that we're moving in the right direction.

In the next few days, we'll be releasing alpha.1, which will fix issues with TypeScript complaining that some commands, plugins, and configurations are not available or unknown, even though they're registered correctly (more details). In alpha.2 we will address the issue you reported in this comment.

Internally, we are also discussing what we should export from the main package entry points, or in other words, what is our public API and what should be considered internal. Can you share why and how you use MutationObserver and the commands you mentioned? It will give us a better picture of what you and other developers might need for this and other packages.

Inviz commented 1 year ago

@filipsobol I'm having different monkeypatches to ckeditor, like changing the logic that causes fillers to appear, etc. I'm patching _isBogusMutation in mutation observer to ignore mutations in certain types of custom elements. Patching domconverter to not try to change classes on root. Things like that.

For the command i include them to monkeypatch their refresh method. I guess i could do that during ckeditor initialization instead.

Inviz commented 1 year ago

Anyway you guys are really moving to right direction. Thanks for all the typescript conversion work, probably took a hell of a lot of time. I dont think many projects did it as seamlessly as you did.

filipsobol commented 1 year ago

@Inviz Unfortunately, this is not the supported way to use the editor, and we have decided not to export the MutationObserver and commands from the main entry points. You can still access them as before, but you'll have to deal with potential TypeScript errors yourself.

We plan to release alpha.1 on Monday.

Reinmar commented 1 year ago

@Inviz Unfortunately, this is not the supported way to use the editor, and we have decided not to export the MutationObserver and commands from the main entry points. You can still access them as before, but you'll have to deal with potential TypeScript errors yourself.

A bit of context here. We decided to treat the index.ts entry points as sort of public API of packages. That will allow us to hide some implementation details of each package or things that we expect may change, while still allowing hacking the editor or its features via a direct access to each package's src/ (but at your own risk).

Inviz commented 1 year ago

yeah i think i can deal with that. i'm hacking it already, so i can hack the types too.

pomek commented 1 year ago

The new alpha release is out: 👉 https://github.com/ckeditor/ckeditor5/releases/tag/v37.0.0-alpha.1.

Inviz commented 1 year ago

There's another issue that is just quasy rellated to this:

When using ckeditor from sources without a build, vitest (being esm-first project) complains about CK5 modules not being esm modules, although they seem to should be in its opinion.

Module /Users/sitecore/Sites/feaas-components/ckeditor5/node_modules/@ckeditor/ckeditor5-utils/src/dom/findclosestscrollableancestor.js:14 
seems to be an ES Module but shipped in a CommonJS package. 
You might want to create an issue to the package "@ckeditor/ckeditor5-utils" 
asking them to ship the file in .mjs extension or add "type": "module" in their package.json.

Is this something that can be resolved?

pomek commented 1 year ago

Adding type: module into package.json would resolve the issue.

I'm wondering if sources should be marked as ES Module too.

I extracted the case to a separate issue: https://github.com/ckeditor/ckeditor5/issues/13673.

filipsobol commented 1 year ago

We have some great news for Vue developers! We just released an alpha version of our Vue integration that includes improved TypeScript support. We hope you'll give it a try and share your feedback to help us make sure it works well before we release a stable version. To install it, run npm install @ckeditor/ckeditor5-vue@alpha.

We hope to have similar news for the React and Angular folks next week.

filipsobol commented 1 year ago

@Inviz I looked into the issues you reported in this comment. I hope my answers below will help.


Breaking: selectable in model writer is set to Node type, but according to docs it's https://ckeditor.com/docs/ckeditor5/latest/api/module_engine_model_selection-Selectable.html this. So it doesnt accept position at this time.

The createSelection in model writer has two signatures (thanks to the “function overloading”). The first one expects a Node followed by PlaceOrOffset, and optional options. The other expects any Selectable except Node and optional options.

You can switch between the two definitions in your IDE using this switch:


Breaking: Plugin doesnt have .set method (for observable props)... it used to work

Please check if you have installed the alpha version of the both core and utils packages. If you have, and it doesn't work as shown in the example below (or like in this playground), please let us know.


EventInfo signature seems to have been simplified from EventInfo\<doc, string> to EventInfo\

We don't try to maintain compatibility with DefinitelyTyped typings, so there may be cases like this here and there. The current signature is EventInfo<TName extends string = string, TReturn = unknown>. The first generic defines the type of the event name, and the other defines its return type.


I could not make assignment to Editor.builtinPlugins to work: Type 'typeof import("/Users/sitecore/Sites/feaas-components/ckeditor5/node_modules/@ckeditor/ckeditor5-link/src/autolink")' is not assignable to type 'PluginConstructor\'

I remember seeing this error before we migrated all plugins to TypeScript. Please check if the link plugin and any other plugins you have installed are also in the alpha version. If they are and you still see the error, please let us know.

I also recommend using override to set builtinPlugins or defaultConfig. The "old" way should still work fine, but the one shown below works better with TypeScript.

// Before
class MyEditor extends ClassicEditor {}
MyEditor.builtinPlugins = [];
MyEditor.defaultConfig = {}

// Now
class MyEditor extends ClassicEditor {
  public static override builtinPlugins = [];
  public static override defaultConfig = {};
}

We switched to the second way of defining built-in plugins because it produces better definitions (.d.ts), where the first approach was ignored.

 


I dont use config often, but i'm getting error on assigning heading. I think this is what Reinmar meant talking about module augmentation?

This is already fixed in the latest alpha.

Inviz commented 1 year ago

@filipsobol Thanks Filip! I can confirm that using your suggestions i managed to fix all the issues I've had. Great job

Inviz commented 1 year ago

@filipsobol Can it be somehow improved that we can jump from types to the code implementation? I think it would be hugely useful at least to jump to compiled js (if not ts). Right now i can only jump to the type it seems in vs code

filipsobol commented 1 year ago

@Inviz This is the default behavior of IDEs for libraries that ship definition files, so it's not something we can do at the library level.

I don't know about other IDEs, but in VS Code you can right click on the element you want to jump to and select "Go to source definition".

Inviz commented 1 year ago

Hey Filip. You are right, "Go to source definition" does it. For some reason I thought it should be "Go to implementation". Thanks!

On Wed, Mar 22, 2023 at 5:01 PM Filip Sobol @.***> wrote:

@Inviz https://github.com/Inviz This is the default behavior of IDEs for libraries that ship definition files, so it's not something we can do at the library level.

I don't know about other IDEs, but in VS Code you can right click on the element you want to jump to and select "Go to source definition".

— Reply to this email directly, view it on GitHub https://github.com/ckeditor/ckeditor5/issues/11704#issuecomment-1479146119, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAAVFBKKN5ADP67RQXTXN3W5K5WVANCNFSM5VDMTC5A . You are receiving this because you were mentioned.Message ID: @.***>