Draft-js: Fix issues with Draft.js when React enables Asynchronous Scheduling

Created on 14 Dec 2017  路  5Comments  路  Source: facebook/draft-js

The Problem

tldr; Draft.js has a bug where selection event handling can lead to a
broken state when using asynchronous React.

More detail;
In Draft, we currently expect every event to be followed by an immediate update. If you have multiple handlers updating the editor for the same events, or any asynchronous batching that delays the update, then you can get editorState race conditions. This is documented in more detail here - https://draftjs.org/docs/advanced-topics-editorstate-race-conditions.html#content

The problem is that now, with async React, we may have many updates happen
before React gets around to rendering. As an example, here is a simplified scenario what could happen if a user types "hello" quickly:

  • input event for 'h' keypress
    -> Draft enqueues update from prev. state of '' to 'h'
    (Note - no re-render yet! state remains '')
  • input event for 'e' keypress
    -> Draft enqueues update from prev. state of '' to 'e'
    (Note - no re-render yet! state remains '')
  • input event for 'l' keypress
    -> Draft enqueues update from prev. state of '' to 'l'
    (Note - no re-render yet! state remains '')
  • input event for 'l' keypress
    -> Draft enqueues update from prev. state of '' to 'l'
    (Note - no re-render yet! state remains '')
  • input event for 'o' keypress
    -> Draft enqueues update from prev. state of '' to 'o'
    (Note - no re-render yet! state remains '')
  • React renders the latest queued update
    -> Draft displays just the character 'o' in the editor.

One way or another, it seems we will need to save the updates in a queue and apply them as a batch. The details of how this will work are still TBD.

big picture discussion high priority

Most helpful comment

I think I know how to build a converter from DOM indices to editor indices which accounts for pending insertions and deletions that haven't yet been processed. This would more or less fix the mouse issue. However, it's not enough.

If you have "hello" and type

w o r d Left l

(with no intermediate rerenders) then you should end up with helloworld. However, since arrow key movement is handled natively, the Left would move you to between "hell" and "o" and then you'd end up with hellloword.

We (+@acdlite, @sebmarkbage) had a conversation over lunch and concluded that unless we want to reimplement all native editing operations, it isn't practical to allow Draft to render asynchronously. We could try to reimplement all native editing operations but that's questionable due to OS idioms, assistive tech interop, and Android Chrome which only vends input events.

The new plan is to figure out how to get Draft to update synchronously without updating any sibling components.

(cc @hellendag since we were talking about this and you might be interested in following along)

All 5 comments

@sophiebits did you have any details to add?

Copying what I previously wrote Flarnie via chat:

Applying this patch will let you see (some) errors that async would cause:

diff --git a/src/component/base/DraftEditor.react.js b/src/component/base/DraftEditor.react.js
--- a/html/shared/draft-js/component/base/DraftEditor.react.js
+++ b/html/shared/draft-js/component/base/DraftEditor.react.js
@@ -463,7 +463,9 @@
    */
   update = (editorState: EditorState): void => {
     this._latestEditorState = editorState;
-    this.props.onChange(editorState);
+    setTimeout(() => {
+      this.props.onChange(this._latestEditorState);
+    }, 500);
   };

   /**

Two examples of things that are broken:

  • if you select something (ex: by double clicking a word) then immediately press backspace, it doesn't delete that selection properly
  • if you select (then wait) then delete something, then immediately move your cursor somewhere else and type, the deletion doesn't happen and your cursor ends up in the wrong place

I believe both of those problems are caused by this code https://github.com/facebook/draft-js/blob/327d9ed8adc56118ca27209e695908afd89ca2d0/src/component/handlers/edit/editOnSelect.js#L27 being wrong.

I think I know how to build a converter from DOM indices to editor indices which accounts for pending insertions and deletions that haven't yet been processed. This would more or less fix the mouse issue. However, it's not enough.

If you have "hello" and type

w o r d Left l

(with no intermediate rerenders) then you should end up with helloworld. However, since arrow key movement is handled natively, the Left would move you to between "hell" and "o" and then you'd end up with hellloword.

We (+@acdlite, @sebmarkbage) had a conversation over lunch and concluded that unless we want to reimplement all native editing operations, it isn't practical to allow Draft to render asynchronously. We could try to reimplement all native editing operations but that's questionable due to OS idioms, assistive tech interop, and Android Chrome which only vends input events.

The new plan is to figure out how to get Draft to update synchronously without updating any sibling components.

(cc @hellendag since we were talking about this and you might be interested in following along)

We did end up fixing this. 馃槑

But what should I do if I have my draft state somewhere in parent component?
Consider following scenario:

  1. You are updating draft state inside some view model e. g. clearing input after sending message.
  2. This triggers React's setState and schedules update. Draft.js editor still have old state in props.
  3. Event listener in Draft.js editor receives key pressed event and processes it using only the state it has. It doesn't know about our change now.
    In some moment we should choose between these two states that we've got concurrently and it is the problem.
Was this page helpful?
0 / 5 - 0 ratings

Related issues

vierno picture vierno  路  3Comments

hs737 picture hs737  路  3Comments

eessex picture eessex  路  3Comments

ufo22940268 picture ufo22940268  路  3Comments

pklavins picture pklavins  路  3Comments