-
Notifications
You must be signed in to change notification settings - Fork 6
Description
Hi, following up from h3js/h3#1113 ❤ I had a quick look over current plugin and have some suggestions to improve.
You can use single app.use
for more performance. While onRequest
/onResponse
/onError
are more convenient, you can merge them into a single middleware:
app.use(async (event, next) => {
// [onRequest]
try {
const body = await next();
// [onResponse]
return body
} catch (error) {
// [onError]
throw error
}
})
Apitally only works with H3 v2 and currently doesn’t support nested apps due to limitations in H3.
This section from the docs, I think, should be fixed when you are migrating from global hooks to middleware. (they always run in the order of registration, and usually the main app should register the plugin anyway to catch everything same as Hono suggestion: "Add the Apitally middleware before any other middleware to ensure it wraps the entire stack.")
const requestBody = Buffer.from(await clonedRequest.arrayBuffer())
Is this line really necessary? Buffer.from
is a Node.js specific API and can make platform support limit.
Also, request cloning and always reading body, imposes big performance penalty.
I would suggest if really necessary, expose it as something like event.context._readApitallyRequestBody = () => {...}
(or better, expose another composable util instead of injection to the context)
(feel free to reach me out in discord if easier @pi0
or discord.h3.dev)