react-flowを使ったアプリでonNodeDragが動かないバグの調査

Published: at 18:23

3行で頼む

再現レポジトリ

https://github.com/takaneko/fata-20210908-reproduce-app

GitHubが動画のアップロードに対応していたと知り、再現動画も置いてみた。 ※ 再現アプリではonNodeDoubleClickイベントをバグ発生の契機にしている。

何が起きたか

react-flowを使ったReactアプリでonNodeDragイベントハンドラを登録すると、ノードをドラッグした時に

TypeError: Cannot assign to read only property 'x' of object '#<Object>'

エラーが発生してドラッグできない状態になってしまった。

エラー内容と発生箇所

TypeError: Cannot assign to read only property 'x' of object '#<Object>'

はそのまま「プロパティxを変更できない」というエラーで、 react-flow/src/components/Nodes/wrapNode.tsxの以下の部分で発生している。

node.position.x += draggableData.deltaX; // ココでエラーになる
node.position.y += draggableData.deltaY;
onNodeDrag(event as MouseEvent, node);

nodeが何らかの理由で変更できない状態になってしまったらしい。

nodeはいつ変更できない状態になったのか

react-flowのコードを調べる

このnodeをreact-flowで定義している場所を見るとuseMemoしてるだけだったので、readonlyエラーが起こるようには見えない。

const node = useMemo(
  () => ({ id, type, position: { x: xPos, y: yPos }, data }),
  [id, type, xPos, yPos, data]
);

※ このnodeの値を直接編集するのってお行儀悪い気がするんだけど、バグとは関係ないのでとりあえず忘れた

自分の書いたコードを調べる

手当たり次第にコメントアウトして再現テストを繰り返したところ、onNodeMouseEnterイベントハンドラをコメントアウトした時にエラーが発生しないことがわかった。

onNodeMouseEnterイベントハンドラの実装

const onNodeMouseEnter = (
  event: React.MouseEvent,
  node: Node<any>
) => {
  dispatch(mouseenter(node));
  ...
}

mouseenterアクションの実装

export const flowSlice = createSlice({
  ...
  mouseenter: (state: FlowState, action: PayloadAction<Node<any>>) => {
    state.hoveredNode = action.payload;
  },
  ...
};

mouseenterイベントの引数nodeをreduxのstateとして登録している。 このnodeはエラー発生箇所で変更しようとしているnodeとイコールで、mouseenterイベントはノードをドラッグする前に絶対に発生しているので、このイベントでimmutableにしているのが原因だとわかった。

バグの修正

elements(react-flowに登録するノード全ての配列)からnodeと同じidの要素を取り出して、それをreduxのstate管理するように変更した。

const onNodeMouseEnter = (
  event: React.MouseEvent,
  node: Node<any>
) => {
  const elem = elements.find((e) => e.id === node.id);
  if (!elem) return;
  dispatch(mouseenter(elem));
  ...
}

取り出したelementはonNodeMouseEnterイベントハンドラのnodeと同じ内容だが、react-flow側で管理しているnodeオブジェクトとは参照が異なるため、onNodeDrag呼び出し時にreact-flow側でnode.positionを変更してもエラーにならない。

まとめ

ライブラリ側のイベントハンドラの引数をそのままstate管理したりすると思わぬバグを踏むことがわかった。

react-flow側のコードが ↓ のようになっていてもいい気がするので、Issue or MR作って聞いてみたい。

const draggingNode = {
  ...node,
  position: {
    x: node.position.x + draggableData.deltaX,
    y: node.position.y + draggableData.deltaY,
  },
};
onNodeDrag(event as MouseEvent, draggingNode);

(パフォーマンス的に今のコードの方がいいのかな？)