Ckeditor5-angular: Incorrect documentation instructions: missing types

[jorroll]

The quick start instructions for this module, as described, do not work. Reproduction here: https://stackblitz.com/edit/angular-x2t14d.

The @ckeditor/ckeditor5-build-classic lacks any bundled type definitions, which prevents typescript from compiling the module.

Because there are also no definitions for the module residing in the DefinitelyTyped repo, users must manually add typings for the module. It would be helpful if this step were documented, if only so that users definitively know that they haven't done something wrong (e.g. forgotten to install a package).

Example:

// ckeditor__ckeditor5-build-classic.d.ts

declare module '@ckeditor/ckeditor5-build-classic' {
  const fake: any;
  export default fake;
}

This minimal typescript definition is enough to make the compilation errors go away, but the imported ClassicEditor still will have no types.

I'll also note, that if someone doesn't know how to add custom typings for typescript modules, this example will not be enough information for them. They will need to consult the typescript docs.

[Mgsy]

cc @ma2ciek

[ma2ciek]

Hi, @thefliik.

Yep, our builds lack TS definition types. There's a ticket about it: https://github.com/ckeditor/ckeditor5/issues/504. Note that this issue occurs only in the strict mode, without the strict mode it should compile correctly (And actually even with the error the code will compile).

Because there are also no definitions for the module residing in the DefinitelyTyped repo, users must manually add typings for the module.

We're still waiting for some contributions 😄 The fact is, that the API is really big and it requires a lot of work to make it usable.

I've done research about JS code checked by JSDoc annotations in the past - https://github.com/ckeditor/ckeditor5/issues/1415. This would allow us to generate types after the https://github.com/Microsoft/TypeScript/issues/7546 is closed. Unfortunately, it's a big task and we don't have enough time right now.

I'm unsure whether we should add some fake type placeholders on the DefinitelyTyped repository. Overall maybe it's a good idea.

[jorroll]

Well for basic usage I'm not sure that type definitions are needed. I just learned of this package today, so obviously I still have a lot to learn, but it appears as though standard usage doesn't involve doing anything with the imported module other than passing it as a variable. i.e. (from the docs)

import * as ClassicEditor from '@ckeditor/ckeditor5-build-classic';

@Component( {
    // ...
} )
export class MyComponent {
    public Editor = ClassicEditor;
    // ...
}

In this case, it seem like simply calling out the lack of type definitions would be all that's needed for most use cases (at least of the angular integration). My own project is in strict mode, but it appears as though an error is also generated when using in stackblitz as well.

Again, as much as anything, referencing the fact that type definitions may be needed, but aren't provided, is useful if only so that users know that they haven't done something wrong (like forget to import a package). When a project explicitly calls out Angular support, it makes me expect to find type definitions (rightly or wrongly). As you say, it's totally reasonable that the project doesn't have type definitions, but it would be helpful to acknowledge their absence. It took me a bit to before I was sure I wasn't doing something wrong.

Obviously not a huge deal, as the lack of type definitions doesn't seem to have a huge impact on this package's usefulness within an Angular app.

[Reinmar]

Unless someone is self-assigning this and planning to work on this in the next 2 weeks, I'm removing this from the iteration.

[Reinmar]

Unless someone is self-assigning this and planning to work on this in the next 2 weeks, I'm removing this from the iteration.

OK, it has a PR in https://github.com/ckeditor/ckeditor5/pull/1927.