Rerendering on widget keyUp / keyDown?

Hey Discourse team,

I’m currently investigating why Babble slows down user input so much while typing. I’ve traced it to the keyUp / keyDown hooks in my composer widget, which do things like listen for submit on enter, and escape to cancel, as well as sending notifications that the user is currently typing.

It looks like those events go into a nodeCallback function in discourse/widgets/hooks, which looks like this (NB that this is simplified a bit):

function nodeCallback(widget, keyDownFn) {
  widget.scheduleRerender(() => keyDownFn(widget));
}

Thing is… I really don’t want to rerender the widget on each keystroke, especially because it triggers a rerender of the whole widget tree:

function scheduleRerender() {
  var widget = this;
  while (widget) {
    // rerender the widget if possible
    widget = widget.parentWidget
  }
}

I’m wondering if it’s preferable here to allow the widget to define which events it wants to rerender on, like so:

  createWidget('my-widget', {
    skipKeyUpRerender: true
  })

or whether we can simply turn off rerendering for keyUp / keyDown events, and have the developer manually call scheduleRerender on those if necessary (since they have the potential to be quite high-volume events)

(I haven’t found any existing Discourse code which utilizes the keyUp / keyDown hooks in widgets, so I guess I’m battle testing them :stuck_out_tongue: :crossed_swords: )

cc @eviltrout

1 Like

Hmm this is tricky. The widget framework is designed to re-render following any input, be it clicking or pressing a key. In general re-rendering widgets is fast, so the thought was you should be able to re-render as fast as the user is typing.

Are your renders slow? You can output how many ms are being taken by adding profileWidget: true to its definition.

2 Likes

My guess would be that the render may be slow because it’s traversing up the widget tree and rerendering the whole chat pane (including all of the posts, which are their own widget), whereas if it rerenders on keyUp I’d love it to just rerender the composer (which I reckon would be quite fast).

- slide in menu <-- (this gets rerendered on composer keyUp)
  - chat pane
     - potentially a whole ton of posts
  - composer

Need to confirm this though; will have a go at profiling this later today / tomorrow.

Okay, I can confirm this. With a small amount of posts loaded in the window, the rendering times are acceptable (40-60ms each for keyUp / keyDown)

However, as more posts are loaded into the window, the rendering time of the widget gradually increases:
50 posts: ~50ms
100 posts: ~70ms
150 posts: ~100ms
200 posts: ~130ms

etc.

There’s certainly a scaling issue here, but it doesn’t really manifest itself except that it’s rerendering on keyUp and keyDown :confused: (I need both for separate events), meaning we get to that 300ms limit of human perception quite quickly.

1 Like

Are the posts in the window using the shadowTree property that our post stream does?

If you set that attribute, the virtual dom will not diff the contents of posts on a typical re-render. It will diff if something inside the post triggers the render (such as an action on a button on it).

You might notice a huge speed improvement if you implement this. Of course, it makes the assumption that posts only change as a result of interacting with them directly.

5 Likes

Cool, I think that’s exactly what I’m looking for!