unjs / nitro

Next Generation Server Toolkit. Create web servers with everything you need and deploy them wherever you prefer.
https://nitro.unjs.io
MIT License
5.89k stars 496 forks source link

fix(hooks): enabled use of useAppConfig() and useRuntimeConfig() within request hooks #2483

Open boenrobot opened 3 months ago

boenrobot commented 3 months ago

If the nitro context isn't already initialized, it will be initialized, rather than error.

๐Ÿ”— Linked issue

โ“ Type of change

๐Ÿ“š Description

When calling useAppConfig(event) or useRuntimeConfig(event) from within a hook, the hook errors, because the "nitro" context option is not initialized yet at that time. It does get initialized later, making it available in middleware and route handlers. This PR extends support to include hooks as well, since although the nitro context option is not initialized, the event is available, so one would expect it to work.

๐Ÿ“ Checklist

pi0 commented 3 months ago

Thanks for PR. Can you please link to a minimal reproduction please? ๐Ÿ™๐Ÿผ

boenrobot commented 3 months ago

Thanks for PR. Can you please link to a minimal reproduction please? ๐Ÿ™๐Ÿผ

What about the changes in the tests?

Specifically, test/fixture/plugins/hooks.ts and test/fixture/routes/hooks.ts.

That's pretty much as minimal of a reproduction as you can get.

Restore src/runtime/config.ts to its current state (as opposed to the state in this PR), while keeping the tests files as in this PR, and you will see the tests fail. Surrounding the hook handler with try {} catch (e) { console.error(e);} will show you a message like TypeError: Cannot read properties of undefined (reading 'runtimeConfig') (although exact message may depend on env; this example is from local nodejs installation). The output is wrong regardless, which is why I didn't add it into the test.

I have this fairly minimal project of mine: https://github.com/boenrobot/nuxt-mikro-orm-module

Where I originally encountered this issue, but even that one is not quite as minimal as the test case changes.