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 ^2.0.0 #973

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

Conversation

VaguelySerious
Copy link

@VaguelySerious VaguelySerious commented May 6, 2024

This fork is now available on NPM until the original maintainers of this repository merge this PR: react-quill-new@3.0.0. I will keep publishing this package with any fixes and updates I discover. Visit the react-quill-new repository for an updated README and instructions.

Report any issues you find in on the react-quill-new issues page.

If you're using my react-quill-new and are getting errors like "Unexpected Token: export" or similar, and are using webpack with babel, make sure you have babel transpile this package by ignoring it in the exclude clause for babel-loader. Because Quill moved to being a native ESM module, this PR does the same, which requires ES5-compatible projects to use babel for it.

==============================================================================

This PR updates the quill dependency in react-quill from an outdated version (1.3.7) to the latest release: 2.0.x, and adds support for typescript@^5.0.0.

The underlying Quill@^2.0.0 release mainly includes security fixes and performance enhancement, but most relevant of all, it replaces a deprecated DOM API that will be dropped from Browsers on July 23, 2024 (source).
This means that the following issues will be addressed:

The code in this PR is based on the (already approved) #967, with the addition of:

  • The PR feedback on Upgrade Quill Dependency to 2.0.0-rc.4 #967 by @adgoncal, @piesuke, and @luin.
  • Updates typescript dev dependency from 3.x to 5.x to be in line with Quill 2.0's typescript version. This ensures tsc runs correctly with the new type import syntax.
  • Fixes typescript compilation warnings and errors by upgrading some type dependencies

Note that Quill >2.0.0 uses ES modules natively, which breaks some of the tooling in this repository. This PR doesn't directly address that. Specifically, the test setup isn't compatible with the ES modules.

Next steps

Once this PR is merged, I suggest releasing it as 2.1.0-beta.1, to allow users to upgrade via NPM without suggesting it's completely stable yet. Given the timeline for the DOM API deprecation, we'll have at least a full month to bring this to users in beta state before it becomes advisable to release a full version.

Before a production release, we should revisit the test setup to ensure compatibility with the ES modules. This could take the form of updating the webpack setup to transpile the Quill dependency, or finding a version of Mocha that supports these natively.

@VaguelySerious
Copy link
Author

VaguelySerious commented May 6, 2024

Build command is now running fine, see build step output:

yarn build                                                                                                                     10s  18.20.2 15:33:18
yarn run v1.22.19
$ npm run build:lib && npm run build:dist && npm run build:css

> react-quill@2.0.0 build:lib
> tsc


> react-quill@2.0.0 build:dist
> webpack

Hash: 95cce68c8ae2eb1f8e21
Version: webpack 4.41.2
Time: 1038ms
Built at: 05/06/2024 3:33:25 PM
            Asset      Size  Chunks             Chunk Names
../lib/index.d.ts  5.05 KiB          [emitted]
   react-quill.js  31.4 KiB       0  [emitted]  main
Entrypoint main = react-quill.js
[11] external {"commonjs":"react","commonjs2":"react","amd":"react","root":"React"} 42 bytes {0} [built]
[12] external {"commonjs":"react-dom","commonjs2":"react-dom","amd":"react-dom","root":"ReactDOM"} 42 bytes {0} [built]
[22] (webpack)/buildin/module.js 497 bytes {0} [built]
[26] ./src/index.tsx 17.6 KiB {0} [built]
[41] (webpack)/buildin/global.js 472 bytes {0} [built]
    + 90 hidden modules

> react-quill@2.0.0 build:css
> cpx 'node_modules/quill/dist/quill.*.css' dist

✨  Done in 4.28s.

@hasangursoy
Copy link

Any estimate when the beta npm package will be available?

@VaguelySerious
Copy link
Author

@hasangursoy I'm not a maintainer of this repository, and I think even they would be hard pressed to give you an estimate. If we don't have a beta release by June, however, I'll release this branch as a fork on npm and will link it here. If you need it sooner than that, I suggest making a fork and package yourself.

@VaguelySerious
Copy link
Author

VaguelySerious commented May 13, 2024

For everyone arriving at this PR, @hasangursoy published this fork as a new npm package (see this issue), and I have my own published fork (with updated typescript type exports) under react-quill-new@3.0.0.
This might be a helpful intermediary solution for anyone testing their rollout for the react-quill upgrade. Note that this package is not endorsed or managed by the maintainers of this repository, and does not include a full test suite, so use at your own risk.

@maulana-kurniawan
Copy link

@VaguelySerious Thanks for the temporary NPM package!

@luc345
Copy link

luc345 commented Jun 14, 2024

If the maintainer has no plans to update Quill, @VaguelySerious, will you continue to update react-quill-new? Thank you!

@VaguelySerious
Copy link
Author

@luc345 Yes, I'll continue to update it for now, at least to keep it stable and compatible with Quill and newer versions of React, though I don't plan on addressing any issues or requests unrelated to basic functioning.

@ubaidrehman97
Copy link

@VaguelySerious, I'm currently using react-quill-new but encountering an error:

"Error: Element type is invalid: expected a string (for built-in components) or a class/function (for composite components) but got: object."

Previously, I was using react-quill, which used Quill 1.x. I switched to react-quill-new because I need to utilize the table module. I'm developing with Next.js. Could you please assist me with this issue? Thank you in advance.

image
image
image
image

@VaguelySerious
Copy link
Author

@ubaidrehman97 Have you tried using ESM's import syntax? See NextJS documentation on issues with using require for ESM modules.
If that doesn't work, what's typeof ReactQuill for you? If it's not function (which seems to be the case) then something is likely wrong with your bundler or import syntax. If you need to use require, try const ReactQuill = require('react-quill-new').default; (note the ".default" at the end).

@VaguelySerious
Copy link
Author

@ParkAsher This likely has nothing to do with react-quill-new, rather, with Quill itself, and you should be able to find the solution by googling the error message. From what I recall offhand, it looks something like adding this code snippet somewhere in your code before you import react-quill-new or quill :

import "highlight.js/styles/monokai-sublime.css";
import hljsCore from "highlight.js/lib/core";
import javascript from "highlight.js/lib/languages/javascript";
// ... your other languages

hljsCore.registerLanguage("javascript", javascript);
// ... your other languages

// Quill will look for hljs on the window object when used with "syntax: true" or
// when using the "Quill.convert" method.
window.hljs = hljsCore;

@VaguelySerious
Copy link
Author

VaguelySerious commented Jul 25, 2024

@cpitzak @hamidrezahanafi Thank you both. Added the code and will publish a beta release on NPM later today after testing

@vingrad
Copy link

vingrad commented Aug 21, 2024

Sorry, but how to install your fork?

npm install VaguelySerious/react-quill --save.
doesn't work, because the package directory (react-quill-new) contains only the package.json and node_modules directory.

@hamidrezahanafi
Copy link

Sorry, but how to install your fork?

npm install VaguelySerious/react-quill --save. doesn't work, because the package directory (react-quill-new) contains only the package.json and node_modules directory.

npm i react-quill-new --save

@MartinSilvaMX
Copy link

Do you know why unordered lists becomes ordered?

<ol><li data-list="bullet">....

@VaguelySerious
Copy link
Author

@MartinSilvaMX That is part of quill@^2.0.0, you can create an issue for the quill repo if you want.

@Jasen-Simedru-SHG
Copy link

I'm getting this error: quill__WEBPACK_IMPORTED_MODULE_1__ is not a constructor in my console.
Looks like it's breaking in the createEditor in index.js.
I'm guessing this is what is being referred to in the original comment with the native ESM modules. I've tried several different approaches, but none of them have worked so far. Has anyone else had this issue? If so, what was the fix?

@VaguelySerious
Copy link
Author

@Jasen-Simedru-SHG Are you using babel-loader with webpack and using the exclude: /node_modules\/(?!(react-quill-new)\/).*/, rule with it?

@unartcs
Copy link

unartcs commented Aug 27, 2024

Hi @VaguelySerious I just installed your fork cuz I was getting 'findDOMNode' warnings.
I have noticed some of the tools don't work properly such as list: bullet and check. Do you have any suggestions on how to fix the issues?

@VaguelySerious
Copy link
Author

@unartcs This fork of react-quill updates quill to >=2.0, which changed a lot of things under the hood. It's very likely the problems you're facing are due to the quill upgrade itself, and I suggest looking in the quill github issue log for help. Some hunches:

  • Using quill plugins that aren't compatible with quill 2.0
  • Custom styling for html that has changed due to the quill 2.0 update
  • Other JS code you might have written that's no longer compatible

@omgoshjosh
Copy link

hey is there a way to move discourse to your fork @VaguelySerious ? i found this error Warning: unstable_flushDiscreteUpdates: Cannot flush updates when React is already rendering. caused by this line: https://github.com/VaguelySerious/react-quill/blame/master/src/index.tsx#L222. not sure if it's a serious issue, it seems benign but wanted to get it out there... in any case, thanks for being a badass.

@VaguelySerious
Copy link
Author

@omgoshjosh I opened an issue page on my forked repo. Could you repost your issue there, preferably with steps for reproduction?

@diegoruelasgalaviz
Copy link

Hey @VaguelySerious This is affecting our project, is there a way we can have this Merged soon or do you have an expected date on it?

@VaguelySerious
Copy link
Author

@diegoruelasgalaviz The maintainers of this repository are not responsive, but I'm trying my best to maintain react-quill-new (the fork based on this PR) at https://github.com/VaguelySerious/react-quill/. You can install react-quill-new via npm and it should be a drop-in replacement with updated quill dependencies.

@leonid-br
Copy link

@VaguelySerious, hi!

I have a React application running on Vite. Today, I installed react-quill-new to get rid of the problem: findDOMNode is deprecated and will be removed in the next major release. However, there’s no result; the error still appears in the console.

Additionally, I get another error: Listener added for a 'DOMNodeInserted' mutation event.

Installed packages:

"react-quill": "^2.0.0"
"react-quill-new": "^3.3.1"

I’m working with yarn, and my React version is "react": "^18.3.1".

The component looks like this:
/* eslint-disable react-hooks/exhaustive-deps */
import { useState, useEffect, FC } from 'react';
import { useTranslation } from 'react-i18next';
import ReactQuill from 'react-quill';
import { toast } from 'react-toastify';

type Props = {
value: string;
onChange: (value: string) => void;
imageHandler?: (file: File) => void;
disabledBtn?: boolean;
placeholder?: string;
};
export const CustomReactQuill: FC = ({ value, onChange, imageHandler, disabledBtn = true, placeholder }) => {
const [fildSize, setFildSize] = useState(new Blob([value]).size);
const { t } = useTranslation();

const newFildSize = new Blob([value]).size;
useEffect(() => {
    if (newFildSize - fildSize > 10485760) {
        toast.error(t('QUILL_ERROR'), {
            position: 'bottom-right',
            autoClose: 5000,
            hideProgressBar: false,
            closeOnClick: true,
            pauseOnHover: true,
            draggable: true,
            progress: undefined,
            theme: 'colored',
        });
    } else {
        setFildSize(newFildSize);
    }
}, [value]);

return (
    <ReactQuill
        value={value}
        onChange={onChange}
        modules={{
            toolbar: {
                container: disabledBtn
                    ? [
                          [{ size: ['small', false, 'large', 'huge'] }],
                          [{ header: 1 }, { header: 2 }],
                          ['bold', 'italic', 'underline', 'strike'],
                          [{ align: [] }],
                          [{ list: 'ordered' }, { list: 'bullet' }],
                          [{ color: ['red', 'green', 'blue', 'black', 'yellow', 'orange', 'purple'] }],
                          [{ script: 'sub' }, { script: 'super' }],
                          ['link', 'image', 'video'],
                          ['clean'],
                      ]
                    : [
                          [{ size: ['small', false, 'large', 'huge'] }],
                          [{ header: 1 }, { header: 2 }],
                          ['bold', 'italic', 'underline', 'strike'],
                          [{ align: [] }],
                          [{ list: 'ordered' }, { list: 'bullet' }],
                          [{ color: ['red', 'green', 'blue', 'black', 'yellow', 'orange', 'purple'] }],
                          [{ script: 'sub' }, { script: 'super' }],
                          ['link'],
                          ['clean'],
                      ],
                handlers: {
                    image: imageHandler,
                },
            },
        }}
        placeholder={placeholder}
    />
);

};

I’d appreciate any help.
Thanks!

@VaguelySerious
Copy link
Author

@leonid-br Please file such issues in the issues section of my repo instead.
In your case, these steps should likely fix your issue:

  • Uninstall react-quill
  • Replace import ReactQuill from 'react-quill'; with import ReactQuill from 'react-quill-new';
  • Make sure your package.json does not have any entries pinning the underlying quill package to an older version, e.g. in the resolutions field.

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.