CKEditor loses track of content when parent component key changes

[nathan-charrois]

Are you reporting a feature request or a bug?

Bug

Demo

Demo 1: Click to reorder: https://codesandbox.io/s/ckeditor-demo-4m8ux

Demo 2: Drag and Drop: https://codesandbox.io/s/ckeditor-demo-tchtz

Expected result

CKEditor is not affected by re-order of list.

Actual result

CKEditor content gets lost when list is re-ordered.

Other details

When a component's key changes, CKEditor loses track of its iFrame content. The issue is specific to the iFrame because I tried a CKEditor plugin called "divarea" which changes CKEditor's iframe into a content editable div and that fixes the bug (demo with divarea). However, we want to retain the iFrame functionality for CSS scoping.

Please see the two attached demos. The first isolates the issue demonstrating how changing the key triggers the bug, and the second demo is a use-case example.

[nathan-charrois]

Added more info and a second demo

[icaroscherma]

Have you tried to use static UUID v4 instead of key={id} ? For some components I use (not ckeditor) I use something like: const random_id = useCallback(uuidv4(), []); So React always will have a random unique ID that's not related to an incremental id that can be changed based on the mapping rules.

[nathan-charrois]

That's interesting @icaroscherma, thanks. I applied your idea to the demo and it appears to resolve the issue: https://codesandbox.io/s/ckeditor-demo-moqm0

I'm not sure I understand why this fixes the problem though. By using useCallback() we're ensuring that a random_id is not being generated for each re-render. But then what's the practical difference between this and using an entities' id?

You mentioned incremental id, but in the demo we were using strings "Item 1", "Item 2", etc as the key value. Is there something special that useCallback is doing here?

Edit: While this resolves the issue, it is causing issues elsewhere in my application. I think what's happening is the unique ID forces React to re initialize the component which then forces CKEditor to reinitialize. But it results in component permanence issues

[icaroscherma]

I stumbled upon an issue like this when I was implementing some DnD (drag n' drop) features with children inputs, as I moved them, the DOM gets a bit crazy, also when I deleted a row from one table but I was trying to make a fade-out effect, the next row was faded out until the current one was deleted, because once I changed the index, the next one would have the same as the one I was deleting.

But once I set a unique id to it, if I "move it around", React knows what are the events and values tied to it. I quoted "move it around" because on JSX there is no "moving it around" it's destructing an object and readding an object elsewhere, this is why React needs to know what exactly was tied to that object, so it can re-create all handlers tied to it.

Ps.: I said key={id} as an example, in your code it's key={`item-${value}`} as you noted.

See this example below so I can illustrate my point:

const [options, setOptions] = useState([
  'Icaro', // Index 0
  'John Doe',  // Index 1
  'Jane Doe',  // Index 2
]);

return (
  <ul>
    {options && options.length > 0 && options.map((option, index) => 
      <li key={`myOption-${index}`}>
        {option}
        <button onClick={() => setOptions(options.filter(x => x !== option))}>
          remove
        </button>
      </li>
    )}
  </ul>
);

If I click remove on John Doe, then Jane Doe will have index 1 instead of 2 in the next re-render. Since CKEditor is not rebuild and re-attached, he thinks that he has the contents for John Doe even when it's supposed to have values/events from Jane Doe. If you attach a random unique id that doesn't rebuild on every re-render, even if you kill John Doe, the editor for Jane Doe will have the same id so the events should keep working fine.

While this resolves the issue, it is causing issues elsewhere in my application << can you elaborate more on that? x)

[Edit] By the way, I noticed that you are using just the data value (without onChange) of CKEditor and you are using React Components instead of hooks... is it just for illustrating your demo or you're actually with Components and you're not using onChange?

[nathan-charrois]

Ah yes, avoid using array indexes as keys as much as possible. Perhaps I didnt make it clear in my example, but I'm not using indexes. Using the entity ids (or in my case item-${value}) as the key is the preferred method.

The absence of onChange and hooks vs components shouldn't have any affect on the bug.

As far as I can tell the bug still stands. Generating a uuid for the key is an interesting pattern but I wouldn't recommend it as a resolution to this bug.

[icaroscherma]

I asked about the hooks vs components because your CKEditor, you doesn't seem to be using data-binding, with components you would need a HoC (Higher Order Component) or Redux to mutate your children properly. Right now, after you "print" data on CKEditor, if you change it, your javascript value (state) will not change as you type.

For instance, you are using:

data={`<p>${value}</p>`}

instead of:

data={value}
onChange={e => setData(e.editor.getData())}

so as you type asdf on your CKEditor, deep down it's going to be data={`<p>${value}asdf</p>`}, even if you delete value as you press backspace/delete, it's just a string inside the editor, it's not binded with your value.

You can notice this if you type something on CKEditor and move to another index, it will return to its original value.

---

Using the entity ids (or in my case item-${value}) as the key is the preferred method.

It falls on the example that I wrote before, as you move, React has bond some values and event-handlers to item-1and once you move it to another index, it's going to be item-3 for instance and another object will be item-1 so it will break react's virtual dom.

Do you want to discuss this elsewhere (more real-time) like Discord?

[nathan-charrois]

It's just a demo for the purposes of demonstrating this bug. It's not necessary to setup further than that, heh.

In my application I have a fully implemented CKEditor with data binding change events, hooks, etc. I've purposely made the demo simple so the CKEditor team and understand what is triggering the bug.

I don't really understand what you're saying about keys. I've setup the keys as per React best practices: https://reactjs.org/docs/lists-and-keys.html

In the example you wrote you're using the array index, which is not recommended.

[nathan-charrois]

Updated both demos so there should be less confusion now.

[icaroscherma]

It's just a demo for the purposes of demonstrating this bug. It's not necessary to setup further than that, heh.

I know, I just asked what kind of bug using uuid-v4 brought to your app and you haven't elaborated more. So I was trying to guess what is wrong with it.

In my application I have a fully implemented CKEditor with data binding change events, hooks, etc. I've purposely made the demo simple so the CKEditor team and understand what is triggering the bug.

What is triggering the bug, in both examples you showed on CodeSandbox, it's that CKEditor loses it's reference (literally) when you shift the positions. If you use immutable, non repeated ids, it should not give you any issues anymore (uuid-v4 it's just an quick way of fixing it).

I don't really understand what you're saying about keys. I've setup the keys as per React best practices: https://reactjs.org/docs/lists-and-keys.html In the example you wrote you're using the array index, which is not recommended.

What I recommended was avoiding it. Using index-${i} or just {i} doesn't change much, this was my point, because you will still rely on array iteration index.

So again...

Edit: While this resolves the issue, it is causing issues elsewhere in my application. I think what's happening is the unique ID forces React to re initialize the component which then forces CKEditor to reinitialize. But it results in component permanence issues

results in component permanence issues << can you elaborate?

[nathan-charrois]

Using index-${i} or just {i} doesn't change much, this was my point, because you will still rely on array iteration index.

For sure, and that's why I'm not using the array index. Whether its part of a string or not. React expects a stable key. Ideally the key comes from the object you are iterating over. Which is what I've done in my demo.

It's not clear to me what you're suggesting and maybe there is some confusion I'm not picking up on. You keep mentioning indexes as if I'm using the array index. I'm not, am I? Everyone agrees, do not use index. Let's get that off the table.

If you want to generate a unique id for your entity using uuid(), that's totally fine. My unique Id comes from the server. Just remember, this unique ID must be assigned to your entity in your data layer. Not inside the UI, and certainly not inside the loop as this would be unstable and force React to remount the component unnecessarily.

Sorry for the confusion. At this point we may be talking past each other. If you see a problem with how I've setup either of the demo's please let me know. From my perspective the demos show the bug well but I'm happy to make additional changes if something isn't clear.

Perhaps you could fork my demo and show how your suggestion of implementing a uuid would fix it.

[nathan-charrois]

I've edited the issue to include another demo. In this demo I've added the CKEditor plugin "divarea". This replaces the CKEditor to a Content Editable div.

The purpose of this demo is to demonstrate how the bug is no longer present when the iFrame is removed.

https://codesandbox.io/s/ckeditor-demo-qzeu1?file=/src/index.js

[icaroscherma]

@NathanCH you're absolutely right.

At home I could test your issue further and I see your point. Sorry for being stubborn as a door. Using divarea plugin or just type="inline" would fix your issue but the CSS of your current application will interfere in the WYSIWYG editor as you already told us.

Although the current behaviour of CKEditor's wrapper it's binding the data to the frame at the moment of the creation, as you can't move <iframe>s around without having to rebuild/reload it (javascript issue), one workaround that I see is deleting and recreating the CKEditor object itself, as memoing it won't probably help. Following this idea, a ROUGH fix would be adding a random key to your CKEditor component, so it will be destroyed/recreated every single render. For instance: key={Math.random(100000)}.

A fork of your code. https://codesandbox.io/s/ckeditor-demo-31mv9

I was hitting the same key before because if I change something in this code, it won't be persisted, and I thought initially that it would be related to this. Sorry again.

[sculpt0r]

Hi @NathanCH ,
I've checked your demos again and I'm sure it's related to ckeditor/ckeditor4#4390.
I describe this issue at point 3 in https://github.com/ckeditor/ckeditor4/issues/4390#issuecomment-752922198.

Also, there is an issue we are working on, that could solve this issue: ckeditor/ckeditor4#4462.
As soon as we finish, we will also update this issue.

[nathan-charrois]

Thanks for taking a look and the extra detail, @sculpt0r. I've subscribed to the working issue.

[machineghost]

Following this idea, a ROUGH fix would be adding a random key to your CKEditor component, so it will be destroyed/recreated every single render. For instance: key={Math.random(100000)}.

Changing my code to:

<CKEditor
  key={Math.random(100000)}
  // ...

fixed my rendering issue ... but obviously if that was meant to be a real fix, it wouldn't exactly be an ideal one :)

I guess the real solution is to wait for @sculpt0r to fix the underlying issue ... but until then, random keys are a great hack fix!

[machineghost]

I take that back: while the random key thing does fix the issue, it also creates a bunch more new problems.

I guess I could setup a state variable for the key, and then manually change that state variable when my data changes, but ... given that this hasn't been fixed for over half a year, and doesn't show any sign of getting fixed anytime soon ... it seems I'd be better served looking for a different React text editor component instead.

[nathan-charrois]

Let me know what you decide on @machineghost. We are still using CKEditor because we find it to be years ahead of the competition. Though this bug is still haunting us! Haha.

[icaroscherma]

I switched to SunEditor and I have no regrets. ;) The main thing that I look for and it's somewhat hard to find it's spreadsheet support, luckily SunEditor has it. Might worth giving it a shot. https://github.com/mkhstar/suneditor-react

[nathan-charrois]

Very cool. I hadn't heard about this one, thanks @icaroscherma

[machineghost]

I've tried several RTEs, and up until this bug CKE4 was the best (but this is kind of a deal-breaker, and it doesn't seem to have any momentum toward getting fixed). I'll check out SunEditor, thanks @icaroscherma!

[Dumluregn]

Hello guys, just wanted to let you know we are actively working on the issue. As @sculpt0r mentioned above, it's an upstream problem in CKEditor 4 which we are fixing in two PRs: https://github.com/ckeditor/ckeditor4/pull/4463 and https://github.com/ckeditor/ckeditor4/pull/4481. As you can see there, the issue isn't trivial and takes time, but we are optimistic about fixing this after all. Of course it's totally understandable if you prefer to go with another solution for now, but we'd be happy if you give CKEditor another shot after we close this issue. See you later hopefully!

[machineghost]

Those two tickets have been merged or closed since April ... forgive me for saying so, but it doesn't feel like you are:

actively working on the issue

[MMMalik]

The issue described in the original report was fixed via https://github.com/ckeditor/ckeditor4/issues/4462 and released as part of CKEditor v4.17 major release (released on Nov 17th). See also relevant blog post.

Since v2.1 (released 22nd Nov), CKEditor4 React integration defaults to CKEditor v4.17.1, so the original issue is fixed.

The re-order sample will be updated to leverage the new detach/re-attach feature in https://github.com/ckeditor/ckeditor4-react/pull/263. Demo is already available here.

[CKEditorBot]

Closed in https://github.com/ckeditor/ckeditor4-react/pull/263