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

[Bug]: 2.1.15 and up breaks dozens of our tests #4862

Open
1 of 2 tasks
Nantris opened this issue Feb 7, 2024 · 17 comments
Open
1 of 2 tasks

[Bug]: 2.1.15 and up breaks dozens of our tests #4862

Nantris opened this issue Feb 7, 2024 · 17 comments
Labels
Category: Open Source The issue or pull reuqest is related to the open source packages of Tiptap. Type: Bug The issue or pullrequest is related to a bug

Comments

@Nantris
Copy link
Contributor

Nantris commented Feb 7, 2024

Which packages did you experience the bug in?

core

What Tiptap version are you using?

2.1.15

What’s the bug you are facing?

This version and 2.1.16 break a dozen of our tests which seemingly have nothing to do with the only documented change:

fix(core): fix insertContentAt keeping new lines in html content by @bdbch in #4465

The tests that are broken are as diverse as special modes of splitting list items to handling pasted links to inserting lines above the current line.

Even tests which don't seem to use insertContentAt in any way are broken.

What browser are you using?

Chrome

Code example

No response

What did you expect to happen?

Patch versions should never break tests.

Anything to add? (optional)

I really don't want to be needing to rethink complex logic all over our app because of this change caused by an issue that originated almost 2 years ago (#2720)

I tried updating our insertContentAt calls to use preserveWhitespace with 'full', true and false - all result in still broken tests.

Do we have any remedy here besides just not upgrading until we find spare weeks for this? We're never going to have time to fix all of the problems that change caused.

Example of broken output:

    Expected: "<p>https://example.org\"&gt;https://example.org|&lt;/a&gt;&lt;/p&gt;&lt;/li&gt;&lt;/ul&gt;&lt;p&gt;&lt;/p&gt;|</p>"
    Received: "<p>|</p>"

@svenadlung @bdbch

Did you update your dependencies?

  • Yes, I’ve updated my dependencies to use the latest version of all packages.

Are you sponsoring us?

  • Yes, I’m a sponsor. 💖
@Nantris Nantris added Category: Open Source The issue or pull reuqest is related to the open source packages of Tiptap. Type: Bug The issue or pullrequest is related to a bug labels Feb 7, 2024
@Nantris Nantris changed the title [Bug]: 2.1.15 and up break a dozen of our tests [Bug]: 2.1.15 and up breaks dozens of our tests Feb 7, 2024
@Nantris
Copy link
Contributor Author

Nantris commented Feb 7, 2024

Possible reasons the breaking effects are so wide ranging:

@bdbch
Copy link
Contributor

bdbch commented Feb 7, 2024

Could you give me an example on what kind of content you're setting that leads to the wrong output and what the output should actually look like so I can do local tests and fix them?

@Nantris
Copy link
Contributor Author

Nantris commented Feb 7, 2024

@bdbch the trouble is I'm not really sure what information I can share to help you reproduce this given that I can't understand the relation between the documented change and the broken tests.

Is there any reason to believe the package-lock.json changes could be responsible instead of the insertContentAt changes? I assume not since they're a reversion.

I'll share examples shortly but fear they won't be much help.

@Nantris
Copy link
Contributor Author

Nantris commented Feb 7, 2024

I should mention that I can't guarantee 2.2.x is affected due to #4704 blocking, but I assume so.


I'm still at a loss as far as what to share but for now I've noticed that our yarn.lock has conflicting versions of extensions after upgrading to 2.1.15 which makes me think maybe the issue is not related to the insertContentAt changes but rather is related to #4863

-"@tiptap/extension-blockquote@2.1.13", "@tiptap/extension-blockquote@^2.1.13":
-  version "2.1.13"
-  resolved "https://registry.yarnpkg.com/@tiptap/extension-blockquote/-/extension-blockquote-2.1.13.tgz#abf01e3a00d
72434b08be7f3d7e318c7320db486"
-  integrity sha512-oe6wSQACmODugoP9XH3Ouffjy4BsOBWfTC+dETHNCG6ZED6ShHN3CB9Vr7EwwRgmm2WLaKAjMO1sVumwH+Z1rg==
+"@tiptap/core@^2.1.15":
+  version "2.2.2"
+  resolved "https://registry.yarnpkg.com/@tiptap/core/-/core-2.2.2.tgz#7664197dafee890a5f42ba03b50c202bdf1da761"
+  integrity sha512-fec26LtNgYFGhKzEA9+Of+qLKIKUxDL/XZQofoPcxP71NffcmpZ+ZjAx9NjnvuYtvylUSySZiPauY6WhN3aprw==
 
-"@tiptap/extension-bold@2.1.13", "@tiptap/extension-bold@^2.1.13":
-  version "2.1.13"
-  resolved "https://registry.yarnpkg.com/@tiptap/extension-bold/-/extension-bold-2.1.13.tgz#fb0c8916269be61269e4aef
9d1da417daf52b7f1"
-  integrity sha512-6cHsQTh/rUiG4jkbJer3vk7g60I5tBwEBSGpdxmEHh83RsvevD8+n92PjA24hYYte5RNlATB011E1wu8PVhSvw==
+"@tiptap/extension-blockquote@2.1.15":
+  version "2.1.15"
+  resolved "https://registry.yarnpkg.com/@tiptap/extension-blockquote/-/extension-blockquote-2.1.15.tgz#f8080349f77
db099c11bed0bfb18f4b607303c7b"
+  integrity sha512-H7zKKL85ZefhXS0ye3YYCUemStcs3PQLuSznR/T/KX3ds7AMhHbfRx3nRbMYYUT2dHd0DVVrhgdQPu4HhwbZVQ==
 
-"@tiptap/extension-bubble-menu@^2.1.13":
-  version "2.1.13"
-  resolved "https://registry.yarnpkg.com/@tiptap/extension-bubble-menu/-/extension-bubble-menu-2.1.13.tgz#884cd2e4e
0c9586998baac3d0a14621b177f1859"
-  integrity sha512-Hm7e1GX3AI6lfaUmr6WqsS9MMyXIzCkhh+VQi6K8jj4Q4s8kY4KPoAyD/c3v9pZ/dieUtm2TfqrOCkbHzsJQBg==
+"@tiptap/extension-blockquote@^2.1.15":
+  version "2.2.2"
+  resolved "https://registry.yarnpkg.com/@tiptap/extension-blockquote/-/extension-blockquote-2.2.2.tgz#85b7b4465792
1d213375b1e19821fd07d47629ad"
+  integrity sha512-ENCGx/yhNdUQ0epGOeTN4HFeUSfQDK2CQBy2szkQVtzG/Vhv8ExxBWTxHJcMoeSfEVmKag4B506vfRkKH24IMA==
+
+"@tiptap/extension-bold@2.1.15":
+  version "2.1.15"
+  resolved "https://registry.yarnpkg.com/@tiptap/extension-bold/-/extension-bold-2.1.15.tgz#602e6d3d6e61d99da151df7
29c55a43c8176c1a5"
+  integrity sha512-bGaRXldgZaf7fGwuD0nFuwhxOPLhSa9cqkq4zOUHEQ0LAdXAqAtOe7ftF2Er7bHTd9kp6bvVvcNSWKi+PTTApw==
+
+"@tiptap/extension-bold@^2.1.15":
+  version "2.2.2"
+  resolved "https://registry.yarnpkg.com/@tiptap/extension-bold/-/extension-bold-2.2.2.tgz#e9b612d82a4fd0a7722b7357fa6bd92e227ffbc4"
+  integrity sha512-8/KLpPHwO+GXlWsXEION7ppLfFIaSpnw5m2QYXz/LGRK32hzpTavbdXV3rx9+Vu+7Z+0yQF9G/ro1z9dqTQHpw==
+

@Nantris
Copy link
Contributor Author

Nantris commented Feb 7, 2024

@bdbch it appears that at least some of our tests (I think all) are failing because \n and \t can no longer be inserted into the editor programmatically - even by commands.setContent() - although I didn't see any relationship between that command and insertContentAt.

Input HTML of one of our tests sets this content using .setContent:

<p></p><pre><code class="language-js">abc\n\tdef\n\t\t ghi</code></pre><p></p>

But you can see our editor content doesn't reflect the whitespace at all.

Output HTML of editor.getHTML():

<p></p><pre><code class="language-js">abcdefghi</code></pre><p></p>

Incorrect input result:

image

Expected input result:

image

@Nantris
Copy link
Contributor Author

Nantris commented Feb 13, 2024

Friendly bump. I'd hate to let this go on until it becomes more difficult to fix.

I still don't understand the root cause of how the listed changes could cause these effects.

@bdbch
Copy link
Contributor

bdbch commented Feb 15, 2024

Sorry for the late answer. I was already taking a look into this but didn't had time to get finished. I'll get back at this ASAP as the changes I made should not lead to such issues and were not intended.

I'll add tests for the usecases you specified as we lack tests currently that try to add this type of content into the editor currently.

@bdbch
Copy link
Contributor

bdbch commented Feb 15, 2024

Also do you know which version was the last version that was working? Then I could compare the versions up to the point this test would break and figure out where the root cause comes from.

I guess 2.1.14?

@bdbch
Copy link
Contributor

bdbch commented Feb 15, 2024

I added the following test in our latest version:

it('should keep newlines and tabulators', () => {
  cy.get('.tiptap').then(([{ editor }]) => {
    editor.commands.insertContent('<p>Hello\n\tworld\n\t\thow\n\t\t\tnice.</p>')
    cy.get('.tiptap').should('contain.html', '<p>Hello\n\tworld\n\t\thow\n\t\t\tnice.</p>')
  })
})

and it seems to work I think?

image

Same with newlines and tabulators inside pre tags:

image

I guess that's how it is supposed to look like?

@Nantris
Copy link
Contributor Author

Nantris commented Feb 15, 2024

@bdbch thanks so much for your reply!

Yes 2.1.14 was unaffected.

But, our tests rely on setContent rather than insertContent, which is what makes this particularly strange - since I can't find any relation between the changes and the setContent function.

So for example, to setup our test's initial state, we'll run:

window.editor.chain().setMeta('addToHistory', false).setContent(html).run()


I guess that's how it is supposed to look like?

Indeed, that's the intention.


Because I'm deep in the weeds at the moment I can't readily test what effect changing that to .insertContent(html) on our side might have.

Does your test's result change if you use .setContent?

@bdbch
Copy link
Contributor

bdbch commented Feb 15, 2024

I'll try your case with a set content test

@bdbch
Copy link
Contributor

bdbch commented Feb 16, 2024

Yeah I'm running into the same problem. I'll check out what's happening.

@bdbch
Copy link
Contributor

bdbch commented Feb 16, 2024

For your case I created a fix at #4895 which would reuse the same logic insertContent is using. In theory that should help.

@Nantris
Copy link
Contributor Author

Nantris commented Feb 17, 2024

Thanks very much for your attention on this @bdbch!

@Nantris
Copy link
Contributor Author

Nantris commented Mar 4, 2024

@bdbch did you have any thoughts on how this might be integrated? Are we going to have to wait for 2.3.0? We're already facing other breaking changes in 2.2.x and I'm afraid that if this is pushed off another 3, 6, or 9 months that more problems will accumulate by the time 2.3.x lands with these changes, and we'll effectively be locked into 2.1.14 indefinitely.

I still think reverting 2.1.14 is the best approach, but I don't know how that interplays with 2.2.x I guess reverting it only in 2.1.x would be reasonable since it could be listed as a breaking change in 2.2.x and above - but then when you merge #4895 that will be another breaking change in 2.3.x in the reverse direction of the first.

Complicated... I wonder if you have any thoughts?

@Nantris
Copy link
Contributor Author

Nantris commented Apr 4, 2024

Any update here? We could really use some clarity about what our upgrade path is, or if we're ever even going to have one.

@Nantris
Copy link
Contributor Author

Nantris commented Apr 25, 2024

@svenadlung @bdbch I do apologize for being a pain on this. Could either of you provide any update here? I've managed to fix many of our tests to work around this, but some simply cannot be fixed and so this remains a complete blocker for us for all upgrades; patch, minor, or major.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Open Source The issue or pull reuqest is related to the open source packages of Tiptap. Type: Bug The issue or pullrequest is related to a bug
Projects
Status: Triage open
Development

No branches or pull requests

2 participants