-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ref: Use consistent console instrumentation #8879
Conversation
size-limit report 📦
|
@@ -29,51 +35,50 @@ export class CaptureConsole implements Integration { | |||
/** | |||
* @inheritDoc | |||
*/ | |||
public setupOnce(_: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's not change the signature of this. This integration is part of the public API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be fine for an API as any caller can still pass in anything that you passed into it previously (it accepts more, not less), but I can leave it as is for sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there's no reason to widen the API let's not do it.
790250f
to
7c8586d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if using a Proxy will help remove the fact that our current console instrumentation overrides the filename that is displayed in the console.
I guess we can look at that for v8, maybe? Or have two implementations, but that is a bit bloaty 🤔 |
While looking into logger issues, I noticed that we fill console.xxx multiple times. This PR changes that so that we use the console instrumentation from utils in all cases.