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
55 changes: 36 additions & 19 deletions src/hooks/useEditorView.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,23 @@
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
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 Down Expand Up @@ -42,9 +47,10 @@
* the Editor View unmodified after we upgrade to React 18, which batches every
* update by default.
*/
function withBatchedDispatch(
function withConditionalFlushDispatch(
props: EditorProps,
forceUpdate: () => void
forceUpdate: () => void,
view: EditorView | null
): EditorProps & {
dispatchTransaction: EditorView["dispatch"];
} {
Expand All @@ -55,10 +61,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,20 +86,22 @@
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"
)
);
Expand Down Expand Up @@ -120,14 +129,14 @@
new EditorView(
{ mount },
{
...editorProps,
...editorPropsRef.current,
state,
}
)
);
return;
}
}, [editorProps, mount, state, view]);
}, [props, mount, state, view]);

useLayoutEffect(() => {
view?.setProps(nonStateProps);
Expand All @@ -137,5 +146,13 @@
if (stateProp) view?.setProps({ state: stateProp });
}, [view, stateProp]);

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

Check warning on line 155 in src/hooks/useEditorView.ts

View workflow job for this annotation

GitHub Actions / check

React Hook useLayoutEffect has a missing dependency: 'forceUpdate'. Either include it or remove the dependency array
tilgovi marked this conversation as resolved.
Show resolved Hide resolved
tilgovi marked this conversation as resolved.
Show resolved Hide resolved

return view;
}
Loading