How to best set globals for both (all) server and client-side templates?

FokkeZB

I wanted to show a dropdown with all categories on every page, but it took my a while to figure out how and it still doesn't feel like the most optimal solution.

I added a plugin so I can use hooks to inject the categories in the payload for the server-side rendering or JSON API output where needed.
Using module.parent.require('templates.js').setGlobal() works for server-side rendered templates, but not for client-side rendered templates because the JSON API responses don't go through the server-side render engine.
Using res.locals in a custom plugin works for for server-side rendered templates except the header and footer. These bypass Express some way, which normally merges res.locals with the passed object to render. It again also doesn't work for client-side rendering of course.
What I ended up doing is modifying middleware.js to merge res.locals in the object send to res.json().

@psychobunny, I'm sure you have a more efficient way to do this?

julian

We've discussed this internally in the past, and came to the decision to not merge res.locals into the API returns because templates.js parses a template by iterating through all values in the API return, and replacing them as needed in the template.

This method does mean that the larger the payload, the slower the template parsing.

Plus I believe we also store some private data in res.locals...

@psychobunny?

psychobunny

If I'm not mistaken I think res.locals has a crapton of data that no templates really need. It would be nice to maybe emulate res.locals (ex. Set everything to res.data or something instead?)

FokkeZB

Or widget a hook that allows me to change the variables before they get send out to res.render() or res.json(). If that would come with information on the route/template then I can decide what changes to make.

psychobunny

I would rather if we built the correct hook in the specific function you want to alter data.

It sounds like a pain in the ass today but it makes the software better in the long run imo. If you make an issue for a filter hook we will definitely be happy to add it

FokkeZB

@psychobunny do you mean you'd like to fire a hook in every controller before calling res.render()? That wouldn't be really DRY would it? Since NodeBB is overriding Express's res.render() anyway, I'd add fire a hook there and pass the template name plus the variables. Then the plugin listening on that hook can determine what to do based on the template name,

Where do you want me to request this feature?

I think this is a how it would work, though I must say I just coded this on GitHub, no testing:

Add hook to modify context before render by FokkeZB · Pull Request #2936 · NodeBB/NodeBB

https://community.nodebb.org/topic/4082/how-to-best-set-globals-for-both-all-server-and-client-side-templates

GitHub (github.com)

FokkeZB

The hook might also need to be fired for the rendering of the header and footer since I don't believe that goes through the modified res.render()?

NodeBB/src/middleware/middleware.js at patch-2 · FokkeZB/NodeBB

Node.js based forum software built for the modern web - NodeBB/src/middleware/middleware.js at patch-2 · FokkeZB/NodeBB

GitHub (github.com)

NodeBB/src/middleware/middleware.js at patch-2 · FokkeZB/NodeBB

Node.js based forum software built for the modern web - NodeBB/src/middleware/middleware.js at patch-2 · FokkeZB/NodeBB

GitHub (github.com)

psychobunny

I mean to say that its better to do for example filter:topics.get

Than to have a plugin that looks for filter.render that has logic to check for if its the topic route. This means that every res.render call will make this check, and if you imagine after adding X plugins the overhead can add up

psychobunny

Global wise though, this makes more sense to me than a hook:

If I'm not mistaken I think res.locals has a crapton of data that no templates really need. It would be nice to maybe emulate res.locals (ex. Set everything to res.data or something instead?)

FokkeZB

@psychobunny the thing is that a lot of controllers (e.g. users: https://github.com/FokkeZB/NodeBB/blob/patch-2/src/controllers/users.js) don't have hooks.

Since it's Express's default behaviour to merge res.locals I wouldn't know why you shouldn't apart from that with NodeBB this will end up on the client side as well if the template will be rendered there. So yes, then maybe res.globals would make sense to allow a plugin to set global variables for all templates to receive. But then.. that wouldn't allow you to do so only for specific templates. Which again means overhead.