Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update quill dependency to v2 #711

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

william-gooch
Copy link

A similar PR to #507 but for the latest version of react-quill, with no mixin.

Quill v2, while still being in development, has support for features such as tables and increased support for newer versions of Safari. While this might not be merged in until Quill v2 releases properly, at the moment it's useful to have it in some special cases.

@alexkrolick
Copy link
Collaborator

It looks like the API changes are small enough that we could potentially allow either version of Quill with some minor refactoring

@zenoamaro
Copy link
Owner

zenoamaro commented Aug 18, 2021

@alexkrolick we bundle Quill, so shipping two versions is not straightforward.

Quill is currently in -dev, and I am not sure about how much the API is gonna change. There's no documentation for it (that I am aware of) so forcing it on users as a default seems premature.

But I'd be ok adding support for it in either react-quill v1 and v2—with the user overriding the dependency manually via webpack.

Are you aware of any issue preventing release of v2 so far?

@alexkrolick
Copy link
Collaborator

alexkrolick commented Aug 18, 2021

Screen Shot 2021-08-18 at 4 07 25 PM

But I'd be ok adding support for it in either react-quill v1 and v2—with the user overriding the dependency manually via webpack.

Yep, I was thinking bundling Quill v1 but handling v2 if it is installed via a package.json override. Quill.version appears to be set so we can use that to switch. The only trouble is DeltaStatic was moved to a different package (quill-delta) and Quill doesn't re-export it, so we'd need to do a synchronous require in order to not break the API EDIT: https://github.com/quilljs/quill/blob/develop/core/quill.js#L446-L453 Quill.imports has a reference to it, and list quill-delta and quill@2.0.0-dev.4 in optionalDependencies.

Looking at the issues, I don't see anything specific to our (react-quill)'s v2 beta, so I'd say it's ok to release as stable now and then do a 2.1.0 release later with the support for quill v2. There is some risk the v2 version eventually breaks react-quill in a larger way that would either another major version that drops v1, or a more complex workaround strategy.

@dzienisz
Copy link

Are you still working on V2?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants