Skip to content

Commit f56f942

Browse files
rickhanloniimeta-codesync[bot]
rickhanlonii
authored and
meta-codesync[bot]
committed
Remove ComponentStack → Stack conversion (#55228)
Summary: Pull Request resolved: #55228 LogBox was doing a wasteful round-trip conversion: parseComponentStack converted Stack → ComponentStack, stored it that way, then convertComponentStackToStack converted it back every time it was used. This eliminates the unnecessary conversion by storing componentStack as Stack directly from the start. The ComponentStack type is removed entirely since it's no longer needed. Also consolidates duplicate helper functions in LogBoxLog-test.js by removing createStackForComponentStack and createComponentStack, keeping only the createStack helper. Changelog: [Internal] Reviewed By: vzaidman Differential Revision: D90896380 fbshipit-source-id: d7a0bd07463760470dba7a1aa45df870c1ecd10a
1 parent 7eb3428 commit f56f942

12 files changed

Lines changed: 283 additions & 337 deletions

‎packages/react-native/Libraries/LogBox/Data/LogBoxData.js‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,8 @@
1010

1111
import type {ExtendedError} from '../../Core/ExtendedError';
1212
import type {LogLevel} from './LogBoxLog';
13-
import type {
14-
Category,
15-
ComponentStack,
16-
ExtendedExceptionData,
17-
Message,
18-
} from './parseLogBoxLog';
13+
import type {Stack} from './LogBoxSymbolication';
14+
import type {Category, ExtendedExceptionData, Message} from './parseLogBoxLog';
1915

2016
import DebuggerSessionObserver from '../../../src/private/devsupport/rndevtools/FuseboxSessionObserver';
2117
import toExtendedError from '../../../src/private/utilities/toExtendedError';
@@ -30,7 +26,7 @@ export type LogData = Readonly<{
3026
level: LogLevel,
3127
message: Message,
3228
category: Category,
33-
componentStack: ComponentStack,
29+
componentStack: Stack,
3430
stack?: string,
3531
}>;
3632

‎packages/react-native/Libraries/LogBox/Data/LogBoxLog.js‎

Lines changed: 18 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -9,58 +9,21 @@
99
*/
1010

1111
import type {Stack} from './LogBoxSymbolication';
12-
import type {
13-
Category,
14-
CodeFrame,
15-
ComponentStack,
16-
Message,
17-
} from './parseLogBoxLog';
12+
import type {Category, CodeFrame, Message} from './parseLogBoxLog';
1813

1914
import * as LogBoxSymbolication from './LogBoxSymbolication';
2015

2116
type SymbolicationStatus = 'NONE' | 'PENDING' | 'COMPLETE' | 'FAILED';
2217

2318
export type LogLevel = 'warn' | 'error' | 'fatal' | 'syntax';
2419

25-
// TODO: once component stacks are fully supported, we can refactor
26-
// ComponentStack to just be Stack and remove these conversions fns.
27-
function convertComponentStateToStack(componentStack: ComponentStack): Stack {
28-
return componentStack.map(frame => ({
29-
column: frame?.location?.column,
30-
file: frame.fileName,
31-
lineNumber: frame?.location?.row,
32-
methodName: frame.content,
33-
collapse: false,
34-
}));
35-
}
36-
37-
function convertStackToComponentStack(stack: Stack): ComponentStack {
38-
const componentStack = [];
39-
for (let i = 0; i < stack.length; i++) {
40-
const frame = stack[i];
41-
// NOTE: Skip stack frames missing location.
42-
if (frame.lineNumber != null && frame.column != null) {
43-
componentStack.push({
44-
fileName: frame?.file || '',
45-
location: {
46-
row: frame.lineNumber,
47-
column: frame.column,
48-
},
49-
content: frame.methodName,
50-
collapse: false,
51-
});
52-
}
53-
}
54-
return componentStack;
55-
}
56-
5720
export type LogBoxLogData = Readonly<{
5821
level: LogLevel,
5922
type?: ?string,
6023
message: Message,
6124
stack: Stack,
6225
category: string,
63-
componentStack: ComponentStack,
26+
componentStack: Stack,
6427
codeFrame?: ?CodeFrame,
6528
isComponentError: boolean,
6629
extraData?: unknown,
@@ -71,7 +34,7 @@ class LogBoxLog {
7134
message: Message;
7235
type: ?string;
7336
category: Category;
74-
componentStack: ComponentStack;
37+
componentStack: Stack;
7538
stack: Stack;
7639
count: number;
7740
level: LogLevel;
@@ -89,16 +52,16 @@ class LogBoxLog {
8952
status: 'NONE',
9053
};
9154
symbolicatedComponentStack:
92-
| Readonly<{error: null, componentStack: null, status: 'NONE'}>
93-
| Readonly<{error: null, componentStack: null, status: 'PENDING'}>
55+
| Readonly<{error: null, stack: null, status: 'NONE'}>
56+
| Readonly<{error: null, stack: null, status: 'PENDING'}>
9457
| Readonly<{
9558
error: null,
96-
componentStack: ComponentStack,
59+
stack: Stack,
9760
status: 'COMPLETE',
9861
}>
99-
| Readonly<{error: Error, componentStack: null, status: 'FAILED'}> = {
62+
| Readonly<{error: Error, stack: null, status: 'FAILED'}> = {
10063
error: null,
101-
componentStack: null,
64+
stack: null,
10265
status: 'NONE',
10366
};
10467
onNotificationPress: ?() => void;
@@ -127,9 +90,9 @@ class LogBoxLog {
12790
: this.stack;
12891
}
12992

130-
getAvailableComponentStack(): ComponentStack {
93+
getAvailableComponentStack(): Stack {
13194
return this.symbolicatedComponentStack.status === 'COMPLETE'
132-
? this.symbolicatedComponentStack.componentStack
95+
? this.symbolicatedComponentStack.stack
13396
: this.componentStack;
13497
}
13598

@@ -140,9 +103,7 @@ class LogBoxLog {
140103
retry = true;
141104
}
142105
if (this.symbolicatedComponentStack.status !== 'COMPLETE') {
143-
LogBoxSymbolication.deleteStack(
144-
convertComponentStateToStack(this.componentStack),
145-
);
106+
LogBoxSymbolication.deleteStack(this.componentStack);
146107
retry = true;
147108
}
148109
if (retry) {
@@ -178,15 +139,11 @@ class LogBoxLog {
178139
this.symbolicatedComponentStack.status !== 'COMPLETE'
179140
) {
180141
this.updateComponentStackStatus(null, null, null, callback);
181-
const componentStackFrames = convertComponentStateToStack(
182-
this.componentStack,
183-
);
184-
LogBoxSymbolication.symbolicate(componentStackFrames, []).then(
142+
LogBoxSymbolication.symbolicate(this.componentStack, []).then(
185143
data => {
186144
this.updateComponentStackStatus(
187145
null,
188-
// TODO: we shouldn't need to convert back to ComponentStack any more
189-
convertStackToComponentStack(data.stack),
146+
data.stack,
190147
data?.codeFrame,
191148
callback,
192149
);
@@ -236,30 +193,30 @@ class LogBoxLog {
236193

237194
updateComponentStackStatus(
238195
error: ?Error,
239-
componentStack: ?ComponentStack,
196+
stack: ?Stack,
240197
codeFrame: ?CodeFrame,
241198
callback?: (status: SymbolicationStatus) => void,
242199
): void {
243200
const lastStatus = this.symbolicatedComponentStack.status;
244201
if (error != null) {
245202
this.symbolicatedComponentStack = {
246203
error,
247-
componentStack: null,
204+
stack: null,
248205
status: 'FAILED',
249206
};
250-
} else if (componentStack != null) {
207+
} else if (stack != null) {
251208
if (codeFrame) {
252209
this.componentCodeFrame = codeFrame;
253210
}
254211
this.symbolicatedComponentStack = {
255212
error: null,
256-
componentStack,
213+
stack,
257214
status: 'COMPLETE',
258215
};
259216
} else {
260217
this.symbolicatedComponentStack = {
261218
error: null,
262-
componentStack: null,
219+
stack: null,
263220
status: 'PENDING',
264221
};
265222
}

‎packages/react-native/Libraries/LogBox/Data/__tests__/LogBoxLog-test.js‎

Lines changed: 15 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212

1313
import type {SymbolicatedStackTrace} from '../../../Core/Devtools/symbolicateStackTrace';
1414
import type {StackFrame} from '../../../Core/NativeExceptionsManager';
15-
import type {CodeFrame} from '../parseLogBoxLog';
1615

1716
jest.mock('../LogBoxSymbolication', () => {
1817
return {__esModule: true, symbolicate: jest.fn(), deleteStack: jest.fn()};
@@ -69,25 +68,14 @@ const createStack = (methodNames: Array<string>) =>
6968
methodName,
7069
}));
7170

72-
const createStackForComponentStack = (methodNames: Array<string>) =>
71+
const createComponentStack = (methodNames: Array<string>) =>
7372
methodNames.map((methodName): StackFrame => ({
7473
column: 0,
7574
file: 'file://path/to/component.js',
7675
lineNumber: 1,
7776
methodName,
7877
}));
7978

80-
const createComponentStack = (methodNames: Array<string>) =>
81-
methodNames.map((methodName): CodeFrame => ({
82-
collapse: false,
83-
content: methodName,
84-
location: {
85-
row: 1,
86-
column: 0,
87-
},
88-
fileName: 'file://path/to/component.js',
89-
}));
90-
9179
function mockSymbolicate(
9280
stack: ReadonlyArray<StackFrame>,
9381
stackCodeFrame: ?CodeCodeFrame,
@@ -100,9 +88,7 @@ function mockSymbolicate(
10088
firstFrame.file.indexOf('component.js') > 0
10189
) {
10290
return {
103-
stack: createStackForComponentStack(
104-
stack.map(frame => `C(${frame.methodName})`),
105-
),
91+
stack: createStack(stack.map(frame => `C(${frame.methodName})`)),
10692
codeFrame: COMPONENT_CODE_FRAME,
10793
};
10894
}
@@ -157,7 +143,7 @@ describe('LogBoxLog', () => {
157143
});
158144
expect(log.symbolicatedComponentStack).toEqual({
159145
error: null,
160-
componentStack: null,
146+
stack: null,
161147
status: 'NONE',
162148
});
163149
});
@@ -179,7 +165,7 @@ describe('LogBoxLog', () => {
179165
});
180166
expect(log.symbolicatedComponentStack).toEqual({
181167
error: null,
182-
componentStack: null,
168+
stack: null,
183169
status: 'PENDING',
184170
});
185171

@@ -216,7 +202,7 @@ describe('LogBoxLog', () => {
216202
expect(log.codeFrame).toBe(STACK_CODE_FRAME);
217203
expect(log.symbolicatedComponentStack).toEqual({
218204
error: null,
219-
componentStack: createComponentStack(['C(A)', 'C(B)', 'C(C)']),
205+
stack: createStack(['C(A)', 'C(B)', 'C(C)']),
220206
status: 'COMPLETE',
221207
});
222208
expect(log.componentCodeFrame).toBe(COMPONENT_CODE_FRAME);
@@ -266,7 +252,7 @@ describe('LogBoxLog', () => {
266252
});
267253
expect(log.symbolicatedComponentStack).toEqual({
268254
error: null,
269-
componentStack: createComponentStack(['C(A)', 'C(B)', 'C(C)']),
255+
stack: createStack(['C(A)', 'C(B)', 'C(C)']),
270256
status: 'COMPLETE',
271257
});
272258

@@ -318,7 +304,7 @@ describe('LogBoxLog', () => {
318304
expect(log.codeFrame).toBe(STACK_CODE_FRAME);
319305
expect(log.symbolicatedComponentStack).toEqual({
320306
error,
321-
componentStack: null,
307+
stack: null,
322308
status: 'FAILED',
323309
});
324310

@@ -351,7 +337,7 @@ describe('LogBoxLog', () => {
351337
});
352338
expect(log.symbolicatedComponentStack).toEqual({
353339
error: null,
354-
componentStack: null,
340+
stack: null,
355341
status: 'PENDING',
356342
});
357343

@@ -388,7 +374,7 @@ describe('LogBoxLog', () => {
388374
expect(log.codeFrame).toBe(STACK_CODE_FRAME);
389375
expect(log.symbolicatedComponentStack).toEqual({
390376
error: null,
391-
componentStack: createComponentStack(['C(A)', 'C(B)', 'C(C)']),
377+
stack: createStack(['C(A)', 'C(B)', 'C(C)']),
392378
status: 'COMPLETE',
393379
});
394380
expect(log.componentCodeFrame).toBe(COMPONENT_CODE_FRAME);
@@ -432,7 +418,7 @@ describe('LogBoxLog', () => {
432418
});
433419
expect(log.symbolicatedComponentStack).toEqual({
434420
error,
435-
componentStack: null,
421+
stack: null,
436422
status: 'FAILED',
437423
});
438424

@@ -463,7 +449,7 @@ describe('LogBoxLog', () => {
463449
expect(log.codeFrame).toBe(STACK_CODE_FRAME);
464450
expect(log.symbolicatedComponentStack).toEqual({
465451
error: null,
466-
componentStack: createComponentStack(['C(A)', 'C(B)', 'C(C)']),
452+
stack: createStack(['C(A)', 'C(B)', 'C(C)']),
467453
status: 'COMPLETE',
468454
});
469455
expect(log.componentCodeFrame).toBe(COMPONENT_CODE_FRAME);
@@ -502,7 +488,7 @@ describe('LogBoxLog', () => {
502488
});
503489
expect(log.symbolicatedComponentStack).toEqual({
504490
error: null,
505-
componentStack: createComponentStack(['C(A)', 'C(B)', 'C(C)']),
491+
stack: createStack(['C(A)', 'C(B)', 'C(C)']),
506492
status: 'COMPLETE',
507493
});
508494
expect(log.componentCodeFrame).toBe(COMPONENT_CODE_FRAME);
@@ -534,7 +520,7 @@ describe('LogBoxLog', () => {
534520
expect(log.codeFrame).toBe(STACK_CODE_FRAME);
535521
expect(log.symbolicatedComponentStack).toEqual({
536522
error: null,
537-
componentStack: createComponentStack(['C(A)', 'C(B)', 'C(C)']),
523+
stack: createStack(['C(A)', 'C(B)', 'C(C)']),
538524
status: 'COMPLETE',
539525
});
540526
expect(log.componentCodeFrame).toBe(COMPONENT_CODE_FRAME);
@@ -574,7 +560,7 @@ describe('LogBoxLog', () => {
574560
expect(log.codeFrame).toBe(STACK_CODE_FRAME);
575561
expect(log.symbolicatedComponentStack).toEqual({
576562
error,
577-
componentStack: null,
563+
stack: null,
578564
status: 'FAILED',
579565
});
580566

@@ -605,7 +591,7 @@ describe('LogBoxLog', () => {
605591
expect(log.codeFrame).toBe(STACK_CODE_FRAME);
606592
expect(log.symbolicatedComponentStack).toEqual({
607593
error: null,
608-
componentStack: createComponentStack(['C(A)', 'C(B)', 'C(C)']),
594+
stack: createStack(['C(A)', 'C(B)', 'C(C)']),
609595
status: 'COMPLETE',
610596
});
611597
expect(log.componentCodeFrame).toBe(COMPONENT_CODE_FRAME);

0 commit comments

Comments
 (0)