Preserve event listener ordering

[smoores-dev]

As this hook is currently written, event handlers are registered (and therefore executed) "top-down". That is, a parent component's event handler will be executed before its child components'. This makes it impossible for child components to override or intercept events that are handled by their parents!

This PR simply reverses the order in which handlers are registered (newer handlers, registered further down the component tree, are registered first, rather than last), so that the plugin executes them "bottom-up", which matches the expected behavior for event handlers in React/the DOM generally.

[tilgovi]

The ordering here is the result of implementation details of the hooks, isn't it? If so, that worries me a bit.

[tilgovi]

I believe React does something internally to make event-handling more performant and implement their SyntheticEvent wrapper to normalize event objects across browsers. I think they implement a single, delegated handler at the root of the tree and then they do the bubbling themselves. That's basically what we're trying to do here.

Maybe we should do something to handle depth and bubble deliberately. With the work you've got going on in other PRs, we're looking at tracking the ProseMirror positions of the nodes of every React node view. It's possible we could track positions and depths and use that information here to make an event handler registry that can properly bubble events, without any assumptions about the order in which handlers get registered.

I propose we try to tackle this by extracting the plugin from #27 first and make it track position and depth. Then, we have three things we can do that would benefit from the plugin, and we can try to do them in parallel:

• Fix getPos to use the position from state (Replace getPos with useNodePos #30)
• Establish a proper node view hierarchy (the rest of Use a Plugin to assist in nesting node views according to node hierarchy #27)
• Bubble events properly (this PR)

[smoores-dev]

I don't totally see the path from the plugin to event bubbling (yet!), but I agree that starting with just extracting the plugin seems like a good basis for all of this work. Let's do it!

[tilgovi]

I don't totally see it yet either. It's just a hunch.

[smoores-dev]

Ok, so, some thoughts!

Using the useNodePos hook from #30, we could do something like:

// useEditorEventListener.ts

  const pos = useNodePos() // this won't give us anything if used outside of a node view component, and useEditorEventListener can be used anywhere

  useEditorEffect((view) => {
    if (!view) return
  
    const depth = pos === null ? 0 : view.state.doc.resolve(pos).depth;
    registerEventListener(depth, eventType, eventHandler); // we register the event handler with its depth
    return () => unregisterEventListener(eventType, eventHandler);
  }, [eventHandler, eventType, registerEventListener, unregisterEventListener]);

So that later, in the plugin, we can sort handlers by depth first, and then execute them in descending order, so that leaf handlers run before root handlers.

I'm not sure I like this, though. It's too tied to the ProseMirror document structure, which isn't really the bubbling I would expect as a user; I'm writing this in React components, so I would expect the bubbling to be dependent on the React component tree. In part, this should mean that if I have a parent and child React component that are not node view components, the child's event listener should get a chance to run before the parent's.

I also wonder if we should consider the order of useEditorEffects part of the public API, actually. We are, after all, relying on the order that useLayoutEffects get executed by React to implement LayoutGroup in the first place haha. LayoutGroup works specifically because child Layout Effects are executed before their parents'. So I think we could decide on an ordering for Editor Effects (right now its parents first, then children, but I think we could reverse it without issue?), and then rely on that ordering for event listener execution. This honestly feels good to me; then we are relying on React entirely for "bubbling", which is exactly what I would expect as a user.

[tilgovi]

I also wonder if we should consider the order of useEditorEffects part of the public API, actually. We are, after all, relying on the order that useLayoutEffects get executed by React to implement LayoutGroup in the first place haha. LayoutGroup works specifically because child Layout Effects are executed before their parents'.

What you say about layout effects is true, but the registrations aren't one-shot and can have dependencies, so I'm worried there might be update orderings that cause them to end up out of order with respect to the tree order of their components.

[tilgovi]

Ooh, wait... useEditorEventListener doesn't take any dependencies, so maybe this is fine.

[smoores-dev]

Oh, yes! Right :)

[smoores-dev]

It's maybe still awkward; the underlying useEditorEffect does have dependencies (that never change), and so it effectively only gets run on mount. I think this means that different branches of the tree can end up running in essentially an arbitrary order, but within a given branch, children will always run after parents (because if a parent gets mounted/remounted, so do its children).

[tilgovi]

If we're relying on React's ordering, then this is sort of dependent upon #27, right?

[smoores-dev]

Ah, indeed! In the literal case I was using this for, it wouldn't, because I just need one nodeview to override a handler set by a parent non-node view react component, but in the general case, yes!

[tilgovi]

Gotcha, gotcha. I'm gonna focus on #27 first, then, for the moment.