KillerCodeMonkey / ngx-quill

Angular (>=2) components for the Quill Rich Text Editor
MIT License
1.77k stars 259 forks source link

ECMAScript module fix coming in ngx-quill 25/quill 2? #1784

Closed jitterbox closed 9 months ago

jitterbox commented 9 months ago

I attempted to install ngx-quill@25.0.0-quill2.1 and quill@2.0.0.-beta.0 to test a few things. I'm still seeing this upon building my Angular 17 project:

Warning: [proj]\node_modules\ngx-quill\fesm2022\ngx-quill.mjs depends on 'quill/dist/quill.js'. CommonJS or AMD dependencies can cause optimization bailouts. ...

I saw that quill 2.x is finally fully implementing TypeScript... is the expectation that they will also add ECMAScript module support?

Also, I wasn't able to fully build/test due to this quill error. I assume there's no easy workaround. @KillerCodeMonkey do you have connections on the quill team?

KillerCodeMonkey commented 9 months ago

I am using the cjs module of quill because I can not get it working with their esm.

They inlined The svg toolbar Icons there and angulars default builder is Not working with them.

If I use The esm builds All Icons are Just Shown as icon Paths and not the icon itself.

KillerCodeMonkey commented 9 months ago

I Do not have this error with ngx-quill. I already updated my example repository locally and it is working

KillerCodeMonkey commented 9 months ago

@jitterbox

the ngx-quill-example changes (just hacky to get it work): https://github.com/KillerCodeMonkey/ngx-quill-example/tree/feat/quill-2.beta0

luin commented 9 months ago

Hi @KillerCodeMonkey @jitterbox 👋

Wanted to reach out as the maintainer of Quill editor. I'm currently working on Quill 2.0. Before releasing a stable version, I want to ensure that it won't cause any issues for major community projects. So please let me know if there are anything I can help with!

I noticed in the commit that you had to use the umd version. To simplify things and avoid the requirements we currently impose on bundlers, I think it might be possible to inline the svg content (for example, in quill/ui/icons.js, instead of using var _alignLeft = _interopRequireDefault(require("../assets/icons/align-left.svg")), we can do var _alignLeft = '<svg ...>'). Does that make sense?

KillerCodeMonkey commented 9 months ago

Sure if it is working with the angular build system :). Then we World not Lose typings and Do not get warnings about the usage of cjs libs.

And one question: what is the new way to change the default Block Element from p to div?

Thank you :)

luin commented 9 months ago

@KillerCodeMonkey Created a PR for inlining SVG files: https://github.com/quilljs/quill/pull/3950

And one question: what is the new way to change the default Block Element from p to div?

I don't think there are anything changed in 2.0. You can set tagName of Block to div.

KillerCodeMonkey commented 9 months ago

@luin is it possible to create another beta or alpha release for those changes, so i can check it, if it is working?

luin commented 9 months ago

@KillerCodeMonkey I just released a experimental version. Can you give it a try? npm i quill@experimental.

KillerCodeMonkey commented 9 months ago

nice! i will try to test it today (CET) :D

KillerCodeMonkey commented 9 months ago

@luin it seems to work!

v25.0.0-beta.3

KillerCodeMonkey commented 9 months ago

@luin

with the latest beta i can not override the default block element (causing endless loops in angular)

import Quill from 'quill'
import Block from 'quill/blots/block'

class NewBlock extends Block {}
NewBlock.tagName = 'DIV'
Quill.register(NewBlock, true)

and when i want to execute my tests, webpack fails with the quill bundle.

SyntaxError: Unexpected token ':'
      at Object.call (http://localhost:9876/_karma_webpack_/webpack:/node_modules/quill/dist/quill.js:1861:1)
      at __nested_webpack_require_688__ (http://localhost:9876/_karma_webpack_/webpack:/node_modules/quill/dist/quill.js:36:30)
      at eval (webpack://Quill/./node_modules/quill-delta/dist/Delta.js?:9:28)
      at Object.call (http://localhost:9876/_karma_webpack_/webpack:/node_modules/quill/dist/quill.js:1837:1)
      at __nested_webpack_require_688__ (http://localhost:9876/_karma_webpack_/webpack:/node_modules/quill/dist/quill.js:36:30)
      at eval (webpack://Quill/./core/quill.js?:6:69)
      at Module.call (http://localhost:9876/_karma_webpack_/webpack:/node_modules/quill/dist/quill.js:611:1)
      at __nested_webpack_require_688__ (http://localhost:9876/_karma_webpack_/webpack:/node_modules/quill/dist/quill.js:36:30)
      at eval (webpack://Quill/./core.js?:2:69)
      at Module../core.js (http://localhost:9876/_karma_webpack_/webpack:/node_modules/quill/dist/quill.js:539:1)

Both worked with your experimental release. Maybe the new parchment registry changed something?

luin commented 9 months ago

@KillerCodeMonkey Looks like the error stack shows that you were using dist/quill.js. Not sure if it's related but the default entry point is quill.js.

The code you provide actually works on my side. Can you try it on https://v2.quilljs.com/standalone/full? You can change index.js to:

const Block = Quill.import('blots/block')

class NewBlock extends Block {}
NewBlock.tagName = 'DIV'
Quill.register(NewBlock, true)

const quill = new Quill('#editor', {
  modules: {
    syntax: true,
    toolbar: '#toolbar-container',
  },
  placeholder: 'Compose an epic...',
  theme: 'snow',
});
KillerCodeMonkey commented 9 months ago

wait i think it is my fault.. i used ^2.0.0-beta.1 again... i should omit the circumflex right?

KillerCodeMonkey commented 9 months ago

@luin it was the ^ in front of the version... again... i am so dump... one relaxing weekend and my mind is on vacations.

sorry i bothered you with this... everything is working.

KillerCodeMonkey commented 9 months ago

latest beta version of ngx-quill is using quill 2 beta 1

KillerCodeMonkey commented 9 months ago

@jitterbox i will close the issue. you can use the latest 25 beta release :)