How to best set globals for both (all) server and client-side templates?
-
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.localsin a custom plugin works for for server-side rendered templates except the header and footer. These bypass Express some way, which normally mergesres.localswith 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.localsin the object send tores.json().
@psychobunny, I'm sure you have a more efficient way to do this?
-
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...
-
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?)
-
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
-
@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'sres.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:
-
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()?https://github.com/FokkeZB/NodeBB/blob/patch-2/src/middleware/middleware.js#L193
https://github.com/FokkeZB/NodeBB/blob/patch-2/src/middleware/middleware.js#L336
-
I mean to say that its better to do for example
filter:topics.getThan to have a plugin that looks for
filter.renderthat 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 -
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?)
-
@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.localsI 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 mayberes.globalswould 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.
