node-quill-converter icon indicating copy to clipboard operation
node-quill-converter copied to clipboard

convertHtmlToDelta does not create the exact delta that quill works with

Open Xiphe opened this issue 6 years ago • 11 comments

Given I paste <h2>Hello</h2><p>World</p> into a quill instance running in a real browser. getContents will return

{
  ops: [
    { insert: "Hello" },
    { attributes: { header: 2 }, insert: "\n" },
    { insert: "World\n" }
  ]
}

while convertHtmlToDelta returns

{
  ops: [
    { attributes: { header: 2 }, insert: "Hello\n" },
    { insert: "World\n" }
  ]
}

This cause problems with dirty-checks when the contents of quill are used in a <form>.

Will submit a PR.

Xiphe avatar Nov 15 '18 15:11 Xiphe

Hi @Xiphe, Thanks for submitting this issue (and PR). I'll take a look at this shortly and will follow up! Thank you for your patience and work. - Joel

joelcolucci avatar Nov 16 '18 16:11 joelcolucci

@Xiphe I apologize for the delay. I'll be reviewing this, this week!

joelcolucci avatar Nov 27 '18 15:11 joelcolucci

I can confirm the input/output provided for convertHtmlToDelta .

Input:

'<h2>Hello</h2><p>World</p>'

Resulting Output:

{
  "ops": [
    { "attributes": { "header": 2 }, "insert": "Hello\n" }, 
    { "insert": "World" }
  ]
}

@Xiphe Can you provide the following for when you experience the issue in browser?

  • Version of Quill
  • Browser
  • Browser version
  • Operating system

joelcolucci avatar Dec 23 '18 15:12 joelcolucci

Latest Quill Latest Chrome Latest OSX

Xiphe avatar Dec 23 '18 17:12 Xiphe

Hi @Xiphe , Can you send me the semvers for each?

joelcolucci avatar Dec 27 '18 03:12 joelcolucci

Hey @joelcolucci, in the meantime we stopped using this package since there seems to be a memory leak (which we did not debug properly, so I can not tell you where and why).

Versions related to this issue:

"quill": "^1.3.6" macOS: 10.14.1 Chrome: Version 71.0.3578.98 (Official Build) (64-bit)

From my side you can also just close this + #4 - It's up to you.

Xiphe avatar Dec 30 '18 12:12 Xiphe

Thanks @Xiphe . I appreciate the feedback and update. This month I am committing more time to this project.

Are you able to share at what scale the memory leak occurred at? As in converting 1000, 10000, 100000 quills etc?

Thank you again for your time.

joelcolucci avatar Jan 02 '19 13:01 joelcolucci

Are you able to share at what scale the memory leak occurred at.

As in not converting at all but loading the module. But as I said, we have not digged any deeper. Could also be a not relate to this module and we interpreted s.th. wrong.

Maybe @fgnass knows a little more here.

Xiphe avatar Jan 03 '19 00:01 Xiphe

+1 on the memory leak. Just figured that out after simply loading the module.

node.js 10.10.0 npm 6.4.1 express: 4.17.1

jessewilliams1 avatar Jul 17 '19 22:07 jessewilliams1

@joelcolucci Is memory leak issue resolved now? I have to convert HTML to Delta on node server side. So i need this package to implement on node side

paramjeetkumar7297470 avatar Nov 19 '19 16:11 paramjeetkumar7297470

Hey all. Just published version 0.3.3 which should resolve the memory leak from importing the module. See #10 for updates.

joelcolucci avatar Jan 24 '20 00:01 joelcolucci