@samitouri / QOS-React / commits / e8df0cf9f7

Switch to binding the console with badging instead of calling it directly (#30461)

This is a major nit but this avoids an extra stack frame when we're replaying logs. Normally the `printToConsole` frame doesn't show up because it'd be ignore listed. <img width="421" alt="Screenshot 2024-07-25 at 11 49 39 AM" src="https://github.com/user-attachments/assets/81334c2f-e19e-476a-871e-c4db9dee294e"> When you expand to show ignore listed frames a ton of other frames show up. <img width="516" alt="Screenshot 2024-07-25 at 11 49 47 AM" src="https://github.com/user-attachments/assets/2ab8bdfb-464c-408d-9176-ee2fabc114b6"> The annoying thing about this frame is that it's at the top of the stack where as typically framework stuff ends up at the bottom and something you can ignore. The user space stack comes first. With this fix there's no longer any `printToConsole` frame. <img width="590" alt="Screenshot 2024-07-25 at 12 09 09 PM" src="https://github.com/user-attachments/assets/b8365d53-31f3-43df-abce-172d608d3c9c"> Am I wiling to eat the added complexity and slightly slower performance for this nit? Definitely.

Sebastian Markbåge committed Jul 25, 2024 at 12:32 UTC e8df0cf9f7c7f641192f19841db9bf34b6a0abf7
11 files changed +56 -38
packages/react-client/src/ReactClientConsoleConfigBrowser.js
+11 -7
@@ -20,11 +20,13 @@ const badgeStyle =
20 const resetStyle = '';
21 const pad = ' ';
22
23 -export function printToConsole(
23 +const bind = Function.prototype.bind;
24 +
25 +export function bindToConsole(
26 methodName: string,
27 args: Array<any>,
28 badgeName: string,
27 -): void {
29 +): () => any {
30 let offset = 0;
31 switch (methodName) {
32 case 'dir':
@@ -32,9 +34,8 @@ export function printToConsole(
34 case 'groupEnd':
35 case 'table': {
36 // These methods cannot be colorized because they don't take a formatting string.
35 - // eslint-disable-next-line react-internal/no-production-logging
36 - console[methodName].apply(console, args);
37 - return;
37 + // $FlowFixMe
38 + return bind.apply(console[methodName], [console].concat(args)); // eslint-disable-line react-internal/no-production-logging
39 }
40 case 'assert': {
41 // assert takes formatting options as the second argument.
@@ -63,6 +64,9 @@ export function printToConsole(
64 );
65 }
66
66 - // $FlowFixMe[invalid-computed-prop]
67 - console[methodName].apply(console, newArgs); // eslint-disable-line react-internal/no-production-logging
67 + // The "this" binding in the "bind";
68 + newArgs.unshift(console);
69 +
70 + // $FlowFixMe
71 + return bind.apply(console[methodName], newArgs); // eslint-disable-line react-internal/no-production-logging
72 }
packages/react-client/src/ReactClientConsoleConfigPlain.js
+11 -7
@@ -10,11 +10,13 @@
10 const badgeFormat = '[%s] ';
11 const pad = ' ';
12
13 -export function printToConsole(
13 +const bind = Function.prototype.bind;
14 +
15 +export function bindToConsole(
16 methodName: string,
17 args: Array<any>,
18 badgeName: string,
17 -): void {
19 +): () => any {
20 let offset = 0;
21 switch (methodName) {
22 case 'dir':
@@ -22,9 +24,8 @@ export function printToConsole(
24 case 'groupEnd':
25 case 'table': {
26 // These methods cannot be colorized because they don't take a formatting string.
25 - // eslint-disable-next-line react-internal/no-production-logging
26 - console[methodName].apply(console, args);
27 - return;
27 + // $FlowFixMe
28 + return bind.apply(console[methodName], [console].concat(args)); // eslint-disable-line react-internal/no-production-logging
29 }
30 case 'assert': {
31 // assert takes formatting options as the second argument.
@@ -44,6 +45,9 @@ export function printToConsole(
45 newArgs.splice(offset, 0, badgeFormat, pad + badgeName + pad);
46 }
47
47 - // $FlowFixMe[invalid-computed-prop]
48 - console[methodName].apply(console, newArgs); // eslint-disable-line react-internal/no-production-logging
48 + // The "this" binding in the "bind";
49 + newArgs.unshift(console);
50 +
51 + // $FlowFixMe
52 + return bind.apply(console[methodName], newArgs); // eslint-disable-line react-internal/no-production-logging
53 }
packages/react-client/src/ReactClientConsoleConfigServer.js
+11 -7
@@ -21,11 +21,13 @@ const badgeStyle =
21 const resetStyle = '';
22 const pad = ' ';
23
24 -export function printToConsole(
24 +const bind = Function.prototype.bind;
25 +
26 +export function bindToConsole(
27 methodName: string,
28 args: Array<any>,
29 badgeName: string,
28 -): void {
30 +): () => any {
31 let offset = 0;
32 switch (methodName) {
33 case 'dir':
@@ -33,9 +35,8 @@ export function printToConsole(
35 case 'groupEnd':
36 case 'table': {
37 // These methods cannot be colorized because they don't take a formatting string.
36 - // eslint-disable-next-line react-internal/no-production-logging
37 - console[methodName].apply(console, args);
38 - return;
38 + // $FlowFixMe
39 + return bind.apply(console[methodName], [console].concat(args)); // eslint-disable-line react-internal/no-production-logging
40 }
41 case 'assert': {
42 // assert takes formatting options as the second argument.
@@ -64,6 +65,9 @@ export function printToConsole(
65 );
66 }
67
67 - // $FlowFixMe[invalid-computed-prop]
68 - console[methodName].apply(console, newArgs); // eslint-disable-line react-internal/no-production-logging
68 + // The "this" binding in the "bind";
69 + newArgs.unshift(console);
70 +
71 + // $FlowFixMe
72 + return bind.apply(console[methodName], newArgs); // eslint-disable-line react-internal/no-production-logging
73 }
packages/react-client/src/ReactFlightClient.js
+3 -3
@@ -56,7 +56,7 @@ import {
56 readFinalStringChunk,
57 createStringDecoder,
58 prepareDestinationForModule,
59 - printToConsole,
59 + bindToConsole,
60 } from './ReactFlightClientConfig';
61
62 import {registerServerReference} from './ReactFlightReplyClient';
@@ -2313,14 +2313,14 @@ function resolveConsoleEntry(
2313 if (!enableOwnerStacks) {
2314 // Printing with stack isn't really limited to owner stacks but
2315 // we gate it behind the same flag for now while iterating.
2316 - printToConsole(methodName, args, env);
2316 + bindToConsole(methodName, args, env)();
2317 return;
2318 }
2319 const callStack = buildFakeCallStack(
2320 response,
2321 stackTrace,
2322 env,
2323 - printToConsole.bind(null, methodName, args, env),
2323 + bindToConsole(methodName, args, env),
2324 );
2325 if (owner != null) {
2326 const task = initializeFakeTask(response, owner, env);
packages/react-client/src/forks/ReactFlightClientConfig.custom.js
+1 -1
@@ -48,4 +48,4 @@ export const createStringDecoder = $$$config.createStringDecoder;
48 export const readPartialStringChunk = $$$config.readPartialStringChunk;
49 export const readFinalStringChunk = $$$config.readFinalStringChunk;
50
51 -export const printToConsole = $$$config.printToConsole;
51 +export const bindToConsole = $$$config.bindToConsole;
packages/react-noop-renderer/src/ReactNoopFlightClient.js
+6 -3
@@ -45,9 +45,12 @@ const {createResponse, processBinaryChunk, getRoot, close} = ReactFlightClient({
45 parseModel(response: Response, json) {
46 return JSON.parse(json, response._fromJSON);
47 },
48 - printToConsole(methodName, args, badgeName) {
49 - // eslint-disable-next-line react-internal/no-production-logging
50 - console[methodName].apply(console, args);
48 + bindToConsole(methodName, args, badgeName) {
49 + return Function.prototype.bind.apply(
50 + // eslint-disable-next-line react-internal/no-production-logging
51 + console[methodName],
52 + [console].concat(args),
53 + );
54 },
55 });
56
packages/react-noop-renderer/src/createReactNoop.js
+6 -3
@@ -636,9 +636,12 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
636
637 resetFormInstance(form: Instance) {},
638
639 - printToConsole(methodName, args, badgeName) {
640 - // eslint-disable-next-line react-internal/no-production-logging
641 - console[methodName].apply(console, args);
639 + bindToConsole(methodName, args, badgeName) {
640 + return Function.prototype.bind.apply(
641 + // eslint-disable-next-line react-internal/no-production-logging
642 + console[methodName],
643 + [console].concat(args),
644 + );
645 },
646 };
647
packages/react-reconciler/src/ReactFiberErrorLogger.js
+3 -3
@@ -20,7 +20,7 @@ import ReactSharedInternals from 'shared/ReactSharedInternals';
20
21 import {enableOwnerStacks} from 'shared/ReactFeatureFlags';
22
23 -import {printToConsole} from './ReactFiberConfig';
23 +import {bindToConsole} from './ReactFiberConfig';
24
25 // Side-channel since I'm not sure we want to make this part of the public API
26 let componentName: null | string = null;
@@ -117,7 +117,7 @@ export function defaultOnCaughtError(
117 ) {
118 // This was a Server error. We print the environment name in a badge just like we do with
119 // replays of console logs to indicate that the source of this throw as actually the Server.
120 - printToConsole(
120 + bindToConsole(
121 'error',
122 [
123 '%o\n\n%s\n\n%s\n',
@@ -127,7 +127,7 @@ export function defaultOnCaughtError(
127 // We let DevTools or console.createTask add the component stack to the end.
128 ],
129 error.environmentName,
130 - );
130 + )();
131 } else {
132 console.error(
133 '%o\n\n%s\n\n%s\n',
packages/react-reconciler/src/forks/ReactFiberConfig.custom.js
+1 -1
@@ -80,7 +80,7 @@ export const suspendInstance = $$$config.suspendInstance;
80 export const waitForCommitToBeReady = $$$config.waitForCommitToBeReady;
81 export const NotPendingTransition = $$$config.NotPendingTransition;
82 export const resetFormInstance = $$$config.resetFormInstance;
83 -export const printToConsole = $$$config.printToConsole;
83 +export const bindToConsole = $$$config.bindToConsole;
84
85 // -------------------
86 // Microtasks
packages/react-server/src/ReactFizzServer.js
+2 -2
@@ -80,7 +80,7 @@ import {
80 resetResumableState,
81 completeResumableState,
82 emitEarlyPreloads,
83 - printToConsole,
83 + bindToConsole,
84 } from './ReactFizzConfig';
85 import {
86 constructClassInstance,
@@ -386,7 +386,7 @@ function defaultErrorHandler(error: mixed) {
386 ) {
387 // This was a Server error. We print the environment name in a badge just like we do with
388 // replays of console logs to indicate that the source of this throw as actually the Server.
389 - printToConsole('error', [error], error.environmentName);
389 + bindToConsole('error', [error], error.environmentName)();
390 } else {
391 console['error'](error); // Don't transform to our wrapper
392 }
packages/react-server/src/forks/ReactFizzConfig.custom.js
+1 -1
@@ -42,7 +42,7 @@ export const supportsClientAPIs = true;
42 export const supportsRequestStorage = false;
43 export const requestStorage: AsyncLocalStorage<Request | void> = (null: any);
44
45 -export const printToConsole = $$$config.printToConsole;
45 +export const bindToConsole = $$$config.bindToConsole;
46
47 export const resetResumableState = $$$config.resetResumableState;
48 export const completeResumableState = $$$config.completeResumableState;