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

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

Closed
jitterbox opened this issue Jan 8, 2024 · 16 comments
Closed

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

jitterbox opened this issue Jan 8, 2024 · 16 comments
Labels

Comments

@jitterbox
Copy link

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
Copy link
Owner

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
Copy link
Owner

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

@KillerCodeMonkey
Copy link
Owner

@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
Copy link

luin commented Jan 10, 2024

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
Copy link
Owner

KillerCodeMonkey commented Jan 10, 2024 via email

@luin
Copy link

luin commented Jan 11, 2024

@KillerCodeMonkey Created a PR for inlining SVG files: slab/quill#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
Copy link
Owner

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

@luin
Copy link

luin commented Jan 17, 2024

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

@KillerCodeMonkey
Copy link
Owner

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

@KillerCodeMonkey
Copy link
Owner

KillerCodeMonkey commented Jan 17, 2024

@luin it seems to work!

v25.0.0-beta.3

@KillerCodeMonkey
Copy link
Owner

@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
Copy link

luin commented Jan 22, 2024

@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
Copy link
Owner

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

@KillerCodeMonkey
Copy link
Owner

@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
Copy link
Owner

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

@KillerCodeMonkey
Copy link
Owner

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

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

No branches or pull requests

3 participants