slab / quill

Quill is a modern WYSIWYG editor built for compatibility and extensibility
https://quilljs.com
BSD 3-Clause "New" or "Revised" License
43.23k stars 3.36k forks source link

Typescript Errors when Creating Custom Blots #4224

Open gillycheesesteak opened 3 months ago

gillycheesesteak commented 3 months ago

When creating a custom blot, I ran into a confusing situation which I believe can be improved through more explicit documentation or better typescript definitions

Steps for Reproduction

First I tried like this

import Quill from 'quill';

const InlineBlot = Quill.import('blots/inline');

export default class MyCustomBlot extends InlineBlot {...

The return type from Quill.import is unknown, so typescript doesn't like this (even though it works when transpiled)

Then, thinking I needed to import the InlineBlot type directly, I saw that quill does not export types for its blots at the root, so I figured I needed to import from parchment directly

import { InlineBlot } from 'parchment';

export default class MyCustomBlot extends InlineBlot {...

This makes Typescript happy, but this causes issues because the InlineBlot from parchment is not equivalent to blots/inline from quill (this is confusing but I'm sure could be addressed with documentation). The longer explanation is that the custom blot ends up getting erased because of the way the optimize function works on the parent blot, and due to the custom blot extending the wrong subclass

I also tried casting the Quill.import but Typescript wasn't happy about that either

import Quill from 'quill';
import { InlineBlot } from 'parchment';

const Inline = Quill.import('blots/inline') as InlineBlot;

export default class MyCustomBlot extends Inline {...

Typescript gives an error something like Inline is not a constructor, which to be honest I don't understand fully.

Finally, I realized that I can import from the file in quill directly, and this works and satisfies typechecking.

import InlineBlot from 'quill/blots/inline';

export default class MyCustomBlot extends InlineBlot {...

In my opinion, the fact that the quill "enhanced" blots are not accessible as imports from the root of the module gives the impression that they are not meant to be used directly. So while it is not an issue per se, I am left wondering if the documentation or exports could be improved to make this extension feel more natural.

Also, if I am still not doing this the recommended way, an explanation of what that recommended way is would be appreciated and could also be added to documentation.

luin commented 3 months ago

Yes, your last example is the recommended way. We do have a documentation for it: [link](https://quilljs.com/docs/installation#:~:text=Quill.import()%20is%20also%20available%20for%20the%20npm%20build%20to%20access%20Quill%27s%20library.%20However%2C%20a%20more%20natural%20approach%20in%20npm%20enviroment%20is%20to%20import%20the%20formats%20and%20modules%20directly.).

In my opinion, the fact that the quill "enhanced" blots are not accessible as imports from the root of the module gives the impression that they are not meant to be used directly

That's fair. I think it makes sense to export them in the root of the module.

gillycheesesteak commented 3 months ago

Cool, thank you for the link. I think maybe this doc could be updated to make it clear how to accomplish this with proper typechecking (maybe just by updating the Quill.import() to a direct import).

I'm also curious how you envision Quill.import working in general with Typescript in v2. It seems that directly importing might be the recommended approach going forward?

Thank you for your help!

luin commented 3 months ago

I'm also curious how you envision Quill.import working in general with Typescript in v2. It seems that directly importing might be the recommended approach going forward?

Quill.import() is designed to be used in the UMD build, and it's impossible to make it work with TypeScript perfectly due to it connects to a dynamic registry.

It seems that directly importing might be the recommended approach going forward?

Yes. Unless you are not using a bundler, directly importing is the recommended approach.