fastify / point-of-view

Template rendering plugin for Fastify
MIT License
338 stars 86 forks source link

Deoptimized access to `reply.locals` #349

Closed ghost closed 1 year ago

ghost commented 1 year ago

Prerequisites

Plugin version

latest

Description

Not sure if this is a bug or just a docs issue. The docs state:

If you want to provide data, which will be depended on by a request and available in all views, you have to add property locals to reply object ...

But it doesn't look like a decorator is ever added from @fastify/view's side. Is it expected the app manually decorates the reply with locals before registering the plugin, or should the plugin automatically register it?

Theoretically the following code should be somewhere; I just don't know the recommended where 😆:

fastify.decorateReply('locals', null);

fastify.addHook('onRequest', async (request, reply) => {
  reply.locals = {};
});
mcollina commented 1 year ago

Thanks for reporting! Would you like to send a Pull Request to address this issue? Remember to add unit tests.

Adding this in the main plugin function would be ok.

ghost commented 1 year ago

Yeah, I should be able to get a PR later this week! Do you have any pointers on how the tests would work? Since this isn't an error in the sense of explicitly throwing, would I just check for the presence of the decorator, or something else? Not quite sure how to test.

Uzlopak commented 1 year ago

Yes, you basically write a test where you register the plugin and then inject a request and test only if the field is added to the request-object.

ghost commented 1 year ago

Didn't forget about this :D Just a bit busy but hoping to get a PR sent over the weekend.