A tour of how the Widget (Virtual DOM) code in Discourse works

If you change to Glimmer 2, will you still support virtual-dom and old handlebars?

Yes, Glimmer 2 uses the handlebars syntax so the “old handlebars” will just work.

The virtual-dom code is currently not compiled, so Glimmer 2 will expose an AST that I will convert to virtual-dom, so old code will continue to work.

7 Likes

I’m using appEvents to trigger widget re-rendering after message-bus notifications. This is working ok, but I can’t find any way to “de-register” an appEvent listener once a widget is no longer required.

I setup the listener in the widget’s defaultState() function, in the same way as is done in core widgets:

https://github.com/davidtaylorhq/discourse-whos-online/blob/master/assets/javascripts/discourse/initializers/start-whos-online.js.es6#L85-L92

Ideally then I want to be able to call

appEvents.off("whosonline:changed")

when the widget is no longer needed. Otherwise the array of listeners keeps growing and eventually explodes

https://meta.discourse.org/t/whos-online-plugin/52345/90?u=david_taylor

I was hoping to use the widget destroy function, but it doesn’t look like it’s actually wired up to anything. The default implementation is a console message, and I don’t ever see that in the console.

https://github.com/discourse/discourse/blob/master/app/assets/javascripts/discourse/widgets/widget.js.es6#L176-L178

@eviltrout is there some way to run logic when a widget is thrown away? Or is there another way I should be approaching this problem?

4 Likes

Thanks for pointing out that the core widget is watching for an app event in defaultState - that is actually incorrect as it is never unloaded (cc @tgxworld) . In the case of search that might not be a problem since the widget is probably never unmounted but it’s not correct. (I’ve added it to my list to improve.)

The correct way to delegate events to widgets is by using dispatch(). For example:

https://github.com/discourse/discourse/blob/master/app/assets/javascripts/discourse/components/site-header.js.es6#L63

In that case, every time the notifications:changed event is fired, it will call notificationsChanged() in the user-notifications widget. After the method is called, it will automatically queue a rerender.

The event will be automatically removed when the widget is unmounted.

3 Likes

Update, it was definitely a problem, I noticed after using an autocomplete that the event was firing 20+ times! Fixed here:

https://github.com/discourse/discourse/commit/be1cce503c3576426ac9759d88fdcab4c0b87be2

5 Likes

Thanks - that’ll teach me to copy stuff from core :laughing:

So in terms of using dispatch from my plugin, I have modified the parent component that contains the post-avatar widget, to add the dispatch line.

   api.modifyClass('component:scrolling-post-stream', {
     didInsertElement() {
       this._super();
       this.dispatch('whosonline:changed', 'post-avatar');
     }
   });

I discovered that the second argument to dispatch is not the name of the widget, it is in fact the buildKey… This presents a problem for me. I need the event to be distributed to every instance of post-avatar, but obviously each user’s avatar is presented differently, so assigning the same buildKey to all of them makes very weird things happen.

Is implementing something like this the right thing to be do? Register/deregister for the appEvent in the parent component, set dirtyKeys to the buildKeys that require re-rendering, then queue a re-render of the whole post stream widget?

https://github.com/discourse/discourse/blob/1a02f5154fc1d479524e32928f421ebd19b46f6b/app/assets/javascripts/discourse/components/scrolling-post-stream.js.es6#L244-L256

Is there any way for me to set a wildcard dirtyKey, something like post-avatar-*?

2 Likes

Unfortunately you cannot refresh all widgets whose keys match a certain wildcard. You can only refresh all widgets * or do the trick that the scrolling post stream does there with the dirtyKeys if you know the ids of things you are changing.

Your solution of the appEvent in the parent component with the keys should work. I think a cleaner solution would involve re-engineering Discourse’s widget code a bit to add wildcards, or refreshing a widget by name instead of key.

4 Likes

Right, after a lot of headaches I’ve got it working - thanks for the help! :tada:

The two other problems I hit which could possibly do with some re-engineering in the widget code:

  • When setting dirtyKeys, it only re-renders the first widget with that buildKey. I’ve got around that by adding the post-id to the post-avatar’s buildKey so that every buildKey is unique
  • Setting the dirtyKey doesn’t work if the widget you want to re-render is inside another widget. In my case the post-avatar is inside a post. I think this is because of the shadowTree stuff.
    It would be really nice if the widget code could work out that the widget is deep in the tree and re-render its parents. I’ve got around this by setting the post widget as dirty as well.

https://github.com/davidtaylorhq/discourse-whos-online/blob/0437d7f2c8f9e396416e66f0a306b4e9377fc389/assets/javascripts/discourse/initializers/start-whos-online.js.es6#L88-L101

1 Like

That’s because keys are supposed to be unique! So adding the post id to the avatar is correct in this case. I might want to raise an error in development mode if two elements use the same keys. If you want to commit the post avatar key to master that would be fine.

That’s exactly what it is. By default I think a widget is supposed to re-render its parents, but the shadowTree is a performance optimization that avoids too much re-rendering and requires you to be more explicit.

5 Likes

Just in case anyone is trying to track down how widgets can use handlebars templates, see the commit message here (can’t find it documented anywhere on meta):

https://github.com/discourse/discourse/commit/dffb1fc4ee8a4d58f48145decb1590a304e8cf7d

15 Likes

How to create a navigation bar? using widget.

like this

image

The following code in the instruction no longer work. The render works but the state is not incrementing on clicks:

<script type="text/javascript">
        const { createWidget } = require('discourse/widgets/widget');

        createWidget('increment-button', {
            tagName: 'button',

            defaultState() {
                return { clicks: 0 };
            },

            html(attrs, state) {
                return `Click me! ${state.clicks} clicks`;
            },

            click() {
                this.state.clicks++;
            }
        });
</script>


<script type='text/x-handlebars' data-template-name='/connectors/above-footer/increment-button'>
    {{mount-widget widget="increment-button" }}
</script>```

Weird, for me when I try that component it doesn’t render at all because it is missing a key. I’ve updated the documentation above to add a buildKey method and it’s working for me. Try that out.

5 Likes

Thanks @eviltrout I was able to get that to work! Much appreciated

1 Like

How can I render html coming from an argument into the widget? Right now it is escaping the HTML and rendering it as it is.

Also the following example is not working:

It seems like the user variable is null. Do I have to do something to pass user into the hbs from where the widget is mounted?

Found answer to this one:

Answer:

import { createWidget } from 'discourse/widgets/widget';
import RawHtml from "discourse/widgets/raw-html";

createWidget('display-name', {
  tagName: 'div.name',

  html() {
    return new RawHtml({ html: `<div>${this.siteSettings.user_rank_alert_message}</div>` });
  }
});

But I still don’t know how to get user data in a connector hbs where data is not passed in from the outlet.

2 Likes

https://github.com/discourse/discourse/blob/a0bbc346cb5d5b89d1a3efdfa89869349a8b067f/app/assets/javascripts/discourse/app/widgets/widget-dropdown.js#L148
check out the triple curlys, bro :smiley:

3 Likes

Not so fast :wink: :warning: :

https://github.com/ember-template-lint/ember-template-lint/blob/master/docs/rule/no-triple-curlies.md

2 Likes

Instead of triple curlies we have a helper you can use instead:

{{html-safe result}}
10 Likes

Hey @eviltrout,

I noticed in the OP for this you say:

But lower down in this topic, you mention

Would you be willing to correct the OP?

1 Like