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

Modify useEditorView to flushSync all dispatched transactions #49

Merged
merged 12 commits into from
Sep 11, 2023
2 changes: 2 additions & 0 deletions .yarn/versions/290b480a.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
releases:
"@nytimes/react-prosemirror": patch
tilgovi marked this conversation as resolved.
Show resolved Hide resolved
84 changes: 54 additions & 30 deletions src/hooks/useEditorView.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,37 @@
import { useForceUpdate } from "./useForceUpdate.js";

Check failure on line 1 in src/hooks/useEditorView.ts

View workflow job for this annotation

GitHub Actions / check

`./useForceUpdate.js` import should occur after import of `react-dom`
ilyaGurevich marked this conversation as resolved.
Show resolved Hide resolved

import type { EditorState, Transaction } from "prosemirror-state";
import { EditorView } from "prosemirror-view";
import type { DirectEditorProps } from "prosemirror-view";
import { useLayoutEffect, useState } from "react";
import { useLayoutEffect, useRef, useState } from "react";
import { unstable_batchedUpdates as batch } from "react-dom";

import { useForceUpdate } from "./useForceUpdate.js";

function withBatchedUpdates<This, T extends unknown[]>(
fn: (this: This, ...args: T) => void
/**
*
* The view.composing state is when the compositionStart eventListener activates,
* which creates an input method editor (IME) for foreign languages.
* https://prosemirror.net/docs/ref/#view.EditorView.composing
*
* In rare exceptions, we want to dispatch transactions in non-batched form
* when the editorView is in a composing state to maintain DOM parity.
*
* See React 18's documentation on opting out of automatic batching
* https://react.dev/blog/2022/03/08/react-18-upgrade-guide#automatic-batching
*
* Returns a conditionally modified props.dispatchTransaction function
*/
function withConditionalFlushUpdates<This, T extends unknown[]>(
fn: (this: This, ...args: T) => void,
view: EditorView | null
): (...args: T) => void {
return function (this: This, ...args: T) {
batch(() => {
if (view?.composing) {
batch(() => {
fn.call(this, ...args);
});
} else {
fn.call(this, ...args);
});
}
};
}

Expand All @@ -31,20 +50,14 @@
export type EditorProps = Omit<DirectEditorProps, "state"> & EditorStateProps;

/**
* Enhances editor props so transactions dispatch in a batched update.
*
* It is important that changes to the editor get batched by React so that any
* components that dispatch transactions in effects do so after rendering with
* state changes from any previous transaction, so that they may use the latest
* state and not trigger nested transactions.
* Conditionally modifies dispatchTransaction props
*
* TODO(OK-4006): We can remove this helper and pass the direct editor props to
* the Editor View unmodified after we upgrade to React 18, which batches every
* update by default.
* Returns modified DirectEditorProps
*/
function withBatchedDispatch(
function withConditionalFlushDispatch(
props: EditorProps,
forceUpdate: () => void
forceUpdate: () => void,
view: EditorView | null
): EditorProps & {
dispatchTransaction: EditorView["dispatch"];
} {
Expand All @@ -55,10 +68,11 @@
this: EditorView,
tr: Transaction
) {
const batchedDispatchTransaction = withBatchedUpdates(
props.dispatchTransaction ?? defaultDispatchTransaction
const conditionallyFlushedDispatch = withConditionalFlushUpdates(
props.dispatchTransaction ?? defaultDispatchTransaction,
view
);
batchedDispatchTransaction.call(this, tr);
conditionallyFlushedDispatch.call(this, tr);
forceUpdate();
},
},
Expand All @@ -79,24 +93,34 @@
props: EditorProps
): EditorView | null {
const [view, setView] = useState<EditorView | null>(null);
const editorPropsRef = useRef(props);

const forceUpdate = useForceUpdate();

const editorProps = withBatchedDispatch(props, forceUpdate);

const stateProp = "state" in editorProps ? editorProps.state : undefined;
const stateProp =
"state" in editorPropsRef.current
? editorPropsRef.current.state
: undefined;

const state =
"defaultState" in editorProps
? editorProps.defaultState
: editorProps.state;
"defaultState" in editorPropsRef.current
? editorPropsRef.current.defaultState
: editorPropsRef.current.state;

const nonStateProps = Object.fromEntries(
Object.entries(editorProps).filter(
Object.entries(editorPropsRef.current).filter(
([propName]) => propName !== "state" && propName !== "defaultState"
)
);

useLayoutEffect(() => {
editorPropsRef.current = withConditionalFlushDispatch(
props,
forceUpdate,
view
);
}, [props, view, forceUpdate]);

useLayoutEffect(() => {
return () => {
if (view) {
Expand All @@ -120,14 +144,14 @@
new EditorView(
{ mount },
{
...editorProps,
...editorPropsRef.current,
state,
}
)
);
return;
}
}, [editorProps, mount, state, view]);
}, [props, mount, state, view]);

useLayoutEffect(() => {
view?.setProps(nonStateProps);
Expand Down
Loading