Skip to content

Change .call() for modern spread call #4504

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

Change .call() for modern spread call #4504

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b166480
Change .call() for modern spread call
PepijnSenders Feb 4, 2022
ab2d993
Update captureconsole.ts
PepijnSenders Feb 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions packages/integrations/src/captureconsole.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,8 @@ export class CaptureConsole implements Integration {
});
}

// this fails for some browsers. :(
if (originalConsoleLevel) {
Function.prototype.apply.call(originalConsoleLevel, global.console, args);
if (originalConsoleLevel && 'originalConsoleLevel' in global.console) {
global.console[originalConsoleLevel](...args);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution! Unfortunately, we can't accept it, because it both causes a build error and breaks the functionality of the code.

Remember that originalConsoleLevel is a function, which is why the build throws this error:

image

Further, originalConsoleLevel is not a property of global.console. I suspect you're thinking of level instead, which is the name of the method in question ("warn", "error", and so on), which IS a property of global.console. That said, changing originalConsoleLevel in your second line to level also doesn't fix it, because now global.console[level] is the wrapped function, not the original.

All of that said:

  1. I can understand how you got confused, because originalConsoleLevel sounds like it should be a string. I'll give it a better name.

  2. You're not wrong that it can be simplified: it can be changed to originalConsoleLevel.call(global.console, args);, which I will also do.

Cheers!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation, I didn't test or check the types yet (just made this in Github editor as an example). Thanks for taking it over and applying this change, this will save some errors in our sentry :)

}
});
});
Expand Down