Skip to content

Commit 408b38e

Browse files
authored
[compiler] Improve display of errors on multi-line expressions (facebook#34963)
When a longer function or expression is identified as the source of an error, we currently print the entire expression in our error message. This is because we delegate to a Babel helper to print codeframes. Here, we add some checking and abbreviate the result if it spans too many lines.
1 parent 09056ab commit 408b38e

File tree

4 files changed

+43
-50
lines changed

4 files changed

+43
-50
lines changed

compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,28 @@ import {Err, Ok, Result} from './Utils/Result';
1212
import {assertExhaustive} from './Utils/utils';
1313
import invariant from 'invariant';
1414

15+
// Number of context lines to display above the source of an error
16+
const CODEFRAME_LINES_ABOVE = 2;
17+
// Number of context lines to display below the source of an error
18+
const CODEFRAME_LINES_BELOW = 3;
19+
/*
20+
* Max number of lines for the _source_ of an error, before we abbreviate
21+
* the display of the source portion
22+
*/
23+
const CODEFRAME_MAX_LINES = 10;
24+
/*
25+
* When the error source exceeds the above threshold, how many lines of
26+
* the source should be displayed? We show:
27+
* - CODEFRAME_LINES_ABOVE context lines
28+
* - CODEFRAME_ABBREVIATED_SOURCE_LINES of the error
29+
* - '...' ellipsis
30+
* - CODEFRAME_ABBREVIATED_SOURCE_LINES of the error
31+
* - CODEFRAME_LINES_BELOW context lines
32+
*
33+
* This value must be at least 2 or else we'll cut off important parts of the error message
34+
*/
35+
const CODEFRAME_ABBREVIATED_SOURCE_LINES = 5;
36+
1537
export enum ErrorSeverity {
1638
/**
1739
* An actionable error that the developer can fix. For example, product code errors should be
@@ -496,7 +518,7 @@ function printCodeFrame(
496518
loc: t.SourceLocation,
497519
message: string,
498520
): string {
499-
return codeFrameColumns(
521+
const printed = codeFrameColumns(
500522
source,
501523
{
502524
start: {
@@ -510,8 +532,25 @@ function printCodeFrame(
510532
},
511533
{
512534
message,
535+
linesAbove: CODEFRAME_LINES_ABOVE,
536+
linesBelow: CODEFRAME_LINES_BELOW,
513537
},
514538
);
539+
const lines = printed.split(/\r?\n/);
540+
if (loc.end.line - loc.start.line < CODEFRAME_MAX_LINES) {
541+
return printed;
542+
}
543+
const pipeIndex = lines[0].indexOf('|');
544+
return [
545+
...lines.slice(
546+
0,
547+
CODEFRAME_LINES_ABOVE + CODEFRAME_ABBREVIATED_SOURCE_LINES,
548+
),
549+
' '.repeat(pipeIndex) + '…',
550+
...lines.slice(
551+
-(CODEFRAME_LINES_BELOW + CODEFRAME_ABBREVIATED_SOURCE_LINES),
552+
),
553+
].join('\n');
515554
}
516555

517556
function printErrorSummary(category: ErrorCategory, message: string): string {

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-uncalled-function-capturing-mutable-values-memoizes-with-captures-values.expect.md

Lines changed: 1 addition & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -60,29 +60,7 @@ This argument is a function which may reassign or mutate `cache` after render, w
6060
> 22 | // The original issue is that `cache` was not memoized together with the returned
6161
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6262
> 23 | // function. This was because neither appears to ever be mutated — the function
63-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
64-
> 24 | // is known to mutate `cache` but the function isn't called.
65-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
66-
> 25 | //
67-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
68-
> 26 | // The fix is to detect cases like this — functions that are mutable but not called -
69-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
70-
> 27 | // and ensure that their mutable captures are aliased together into the same scope.
71-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
72-
> 28 | const cache = new WeakMap<TInput, TOutput>();
73-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
74-
> 29 | return input => {
75-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
76-
> 30 | let output = cache.get(input);
77-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
78-
> 31 | if (output == null) {
79-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
80-
> 32 | output = map(input);
81-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
82-
> 33 | cache.set(input, output);
83-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
84-
> 34 | }
85-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
63+
8664
> 35 | return output;
8765
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
8866
> 36 | };

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-preserve-memo-deps-mixed-optional-nonoptional-property-chain.expect.md

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -64,20 +64,7 @@ error.todo-preserve-memo-deps-mixed-optional-nonoptional-property-chain.ts:7:25
6464
> 8 | return identity({
6565
| ^^^^^^^^^^^^^^^^^^^^^
6666
> 9 | callback: () => {
67-
| ^^^^^^^^^^^^^^^^^^^^^
68-
> 10 | // This is a bug in our dependency inference: we stop capturing dependencies
69-
| ^^^^^^^^^^^^^^^^^^^^^
70-
> 11 | // after x.a.b?.c. But what this dependency is telling us is that if `x.a.b`
71-
| ^^^^^^^^^^^^^^^^^^^^^
72-
> 12 | // was non-nullish, then we can access `.c.d?.e`. Thus we should take the
73-
| ^^^^^^^^^^^^^^^^^^^^^
74-
> 13 | // full property chain, exactly as-is with optionals/non-optionals, as a
75-
| ^^^^^^^^^^^^^^^^^^^^^
76-
> 14 | // dependency
77-
| ^^^^^^^^^^^^^^^^^^^^^
78-
> 15 | return identity(x.a.b?.c.d?.e);
79-
| ^^^^^^^^^^^^^^^^^^^^^
80-
> 16 | },
67+
8168
| ^^^^^^^^^^^^^^^^^^^^^
8269
> 17 | });
8370
| ^^^^^^^^^^^^^^^^^^^^^

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.todo-syntax.expect.md

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -46,18 +46,7 @@ error.todo-syntax.ts:11:2
4646
> 12 | () => {
4747
| ^^^^^^^^^^^
4848
> 13 | try {
49-
| ^^^^^^^^^^^
50-
> 14 | console.log(prop1);
51-
| ^^^^^^^^^^^
52-
> 15 | } finally {
53-
| ^^^^^^^^^^^
54-
> 16 | console.log('exiting');
55-
| ^^^^^^^^^^^
56-
> 17 | }
57-
| ^^^^^^^^^^^
58-
> 18 | },
59-
| ^^^^^^^^^^^
60-
> 19 | [prop1],
49+
6150
| ^^^^^^^^^^^
6251
> 20 | AUTODEPS
6352
| ^^^^^^^^^^^

0 commit comments

Comments
 (0)