fix(nextjs): Use Proxies to wrap to preserve static methods #7002
Conversation
| return async function (this: unknown, ...args: Parameters<H>): Promise<ReturnType<H>> { | ||
| const req = args[0]; | ||
| return new Proxy(handler, { | ||
| apply: async (wrappingTarget, thisArg, args: Parameters<H>) => { |
There was a problem hiding this comment.
Is this supported, e.g. having an async apply? Just wondering :D I guess we don't even really need it here, though? Or am I missing something?
There was a problem hiding this comment.
Not sure about the term "supported" but from my testing, using an async function just seems to return a promise which is about what I would expect to happen. We definitely need it here though because we wanna await the result so that the events are flushed.
There was a problem hiding this comment.
Ah, good, if it works then 👍 maybe it is just a restriction for get based proxies, I vaguely remember having issues with such a thing in the past. But all good then, nice!
size-limit report 📦
|
| // Log a warning if the user is still manually wrapping their route in `withSentry`. Doesn't work in cases where | ||
| // there's been an intermediate wrapper (like `withSentryAPI(someOtherWrapper(withSentry(handler)))`) but should catch | ||
| // most cases. Only runs once per route. (Note: Such double-wrapping isn't harmful, but we'll eventually deprecate and remove `withSentry`, so | ||
| // best to get people to stop using it.) |
There was a problem hiding this comment.
I decided to remove this because it would have been a pain to get working with proxies and it doesn't hurt anyone not to have this log message.
Fixes #6931
Our (automatic) wrapper functions were hiding away any additional fields users defined on the wrapping targets. This PR fixes this by using
Proxywhich will preserve the rest of the fields.