[compiler] Improve display of errors on multi-line expressions

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.

Author: josephsavona

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

@@ -12,6 +12,28 @@ import {Err, Ok, Result} from './Utils/Result';
 import {assertExhaustive} from './Utils/utils';
 import invariant from 'invariant';
 
+// Number of context lines to display above the source of an error
+const CODEFRAME_LINES_ABOVE = 2;
+// Number of context lines to display below the source of an error
+const CODEFRAME_LINES_BELOW = 3;
+/*
+ * Max number of lines for the _source_ of an error, before we abbreviate
+ * the display of the source portion
+ */
+const CODEFRAME_MAX_LINES = 3;
+/*
+ * When the error source exceeds the above threshold, how many lines of
+ * the source should be displayed? We show:
+ * - CODEFRAME_LINES_ABOVE context lines
+ * - CODEFRAME_ABBREVIATED_SOURCE_LINES of the error
+ * - '...' ellipsis
+ * - CODEFRAME_ABBREVIATED_SOURCE_LINES of the error
+ * - CODEFRAME_LINES_BELOW context lines
+ * 
+ * This value must be at least 2 or else we'll cut off important parts of the error message
+ */
+const CODEFRAME_ABBREVIATED_SOURCE_LINES = 2;
+
 export enum ErrorSeverity {
   /**
    * An actionable error that the developer can fix. For example, product code errors should be
@@ -496,7 +518,7 @@ function printCodeFrame(
   loc: t.SourceLocation,
   message: string,
 ): string {
-  return codeFrameColumns(
+  const printed = codeFrameColumns(
     source,
     {
       start: {
@@ -510,8 +532,25 @@ function printCodeFrame(
     },
     {
       message,
+      linesAbove: CODEFRAME_LINES_ABOVE,
+      linesBelow: CODEFRAME_LINES_BELOW,
     },
   );
+  const lines = printed.split(/\r?\n/);
+  if (loc.end.line - loc.start.line < CODEFRAME_MAX_LINES) {
+    return printed;
+  }
+  const pipeIndex = lines[0].indexOf('|');
+  return [
+    ...lines.slice(
+      0,
+      CODEFRAME_LINES_ABOVE + CODEFRAME_ABBREVIATED_SOURCE_LINES,
+    ),
+    '> ' + ' '.repeat(pipeIndex - 2) + '…',
+    ...lines.slice(
+      -(CODEFRAME_LINES_BELOW + CODEFRAME_ABBREVIATED_SOURCE_LINES),
+    ),
+  ].join('\n');
 }
 
 function printErrorSummary(category: ErrorCategory, message: string): string {

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hoist-optional-member-expression-with-conditional-optional.expect.md

@@ -35,18 +35,7 @@ error.hoist-optional-member-expression-with-conditional-optional.ts:4:23
    3 | function Component(props) {
 >  4 |   const data = useMemo(() => {
      |                        ^^^^^^^
->  5 |     const x = [];
-     | ^^^^^^^^^^^^^^^^^
->  6 |     x.push(props?.items);
-     | ^^^^^^^^^^^^^^^^^
->  7 |     if (props.cond) {
-     | ^^^^^^^^^^^^^^^^^
->  8 |       x.push(props?.items);
-     | ^^^^^^^^^^^^^^^^^
->  9 |     }
-     | ^^^^^^^^^^^^^^^^^
-> 10 |     return x;
-     | ^^^^^^^^^^^^^^^^^
+>    …
 > 11 |   }, [props?.items, props.cond]);
      | ^^^^ Could not preserve existing manual memoization
   12 |   return (

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hoist-optional-member-expression-with-conditional.expect.md

@@ -35,18 +35,7 @@ error.hoist-optional-member-expression-with-conditional.ts:4:23
    3 | function Component(props) {
 >  4 |   const data = useMemo(() => {
      |                        ^^^^^^^
->  5 |     const x = [];
-     | ^^^^^^^^^^^^^^^^^
->  6 |     x.push(props?.items);
-     | ^^^^^^^^^^^^^^^^^
->  7 |     if (props.cond) {
-     | ^^^^^^^^^^^^^^^^^
->  8 |       x.push(props.items);
-     | ^^^^^^^^^^^^^^^^^
->  9 |     }
-     | ^^^^^^^^^^^^^^^^^
-> 10 |     return x;
-     | ^^^^^^^^^^^^^^^^^
+>    …
 > 11 |   }, [props?.items, props.cond]);
      | ^^^^ Could not preserve existing manual memoization
   12 |   return (

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-optional-member-expression-as-memo-dep-non-optional-in-body.expect.md

@@ -29,12 +29,7 @@ error.invalid-optional-member-expression-as-memo-dep-non-optional-in-body.ts:3:2
    2 | function Component(props) {
 >  3 |   const data = useMemo(() => {
      |                        ^^^^^^^
->  4 |     // actual code is non-optional
-     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
->  5 |     return props.items.edges.nodes ?? [];
-     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
->  6 |     // deps are optional
-     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+>    …
 >  7 |   }, [props.items?.edges?.nodes]);
      | ^^^^ Could not preserve existing manual memoization
    8 |   return <Foo data={data} />;

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

@@ -57,35 +57,7 @@ This argument is a function which may reassign or mutate `cache` after render, w
   20 | ): TInput => TOutput {
 > 21 |   return useMemo(() => {
      |          ^^^^^^^^^^^^^^^
-> 22 |     // The original issue is that `cache` was not memoized together with the returned
-     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-> 23 |     // function. This was because neither appears to ever be mutated — the function
-     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-> 24 |     // is known to mutate `cache` but the function isn't called.
-     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-> 25 |     //
-     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-> 26 |     // The fix is to detect cases like this — functions that are mutable but not called -
-     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-> 27 |     // and ensure that their mutable captures are aliased together into the same scope.
-     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-> 28 |     const cache = new WeakMap<TInput, TOutput>();
-     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-> 29 |     return input => {
-     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-> 30 |       let output = cache.get(input);
-     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-> 31 |       if (output == null) {
-     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-> 32 |         output = map(input);
-     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-> 33 |         cache.set(input, output);
-     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-> 34 |       }
-     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-> 35 |       return output;
-     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-> 36 |     };
+>    …
      | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 > 37 |   }, [map]);
      | ^^^^^^^^^^^^ This function may (indirectly) reassign or modify `cache` after render

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.repro-preserve-memoization-inner-destructured-value-mistaken-as-dependency-later-mutation.expect.md

@@ -45,10 +45,7 @@ error.repro-preserve-memoization-inner-destructured-value-mistaken-as-dependency
   18 | function useInputValue(input) {
 > 19 |   const object = React.useMemo(() => {
      |                  ^^^^^^^^^^^^^^^^^^^^^
-> 20 |     const {value} = transform(input);
-     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-> 21 |     return {value};
-     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+>    …
 > 22 |   }, [input]);
      | ^^^^^^^^^^^^^^ Could not preserve existing memoization
   23 |   mutate(object);

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-for-in-loop-with-context-variable-iterator.expect.md

@@ -40,18 +40,7 @@ error.todo-for-in-loop-with-context-variable-iterator.ts:8:2
    7 |   // within a closure, the `onClick` handler of each item
 >  8 |   for (let key in props.data) {
      |   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
->  9 |     key = key ?? null; // no-op reassignment to force a context variable
-     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-> 10 |     items.push(
-     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-> 11 |       <div key={key} onClick={() => data.set(key)}>
-     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-> 12 |         {key}
-     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-> 13 |       </div>
-     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-> 14 |     );
-     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+>    …
 > 15 |   }
      | ^^^^ Support non-trivial for..in inits
   16 |   return <div>{items}</div>;

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-for-of-loop-with-context-variable-iterator.expect.md

@@ -40,18 +40,7 @@ error.todo-for-of-loop-with-context-variable-iterator.ts:8:2
    7 |   // within a closure, the `onClick` handler of each item
 >  8 |   for (let item of props.data) {
      |   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
->  9 |     item = item ?? {}; // reassignment to force a context variable
-     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-> 10 |     items.push(
-     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-> 11 |       <div key={item.id} onClick={() => data.set(item)}>
-     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-> 12 |         {item.id}
-     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-> 13 |       </div>
-     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-> 14 |     );
-     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+>    …
 > 15 |   }
      | ^^^^ Support non-trivial for..of inits
   16 |   return <div>{items}</div>;

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-invalid-jsx-in-catch-in-outer-try-with-finally.expect.md

@@ -35,22 +35,7 @@ error.todo-invalid-jsx-in-catch-in-outer-try-with-finally.ts:6:2
    5 |   let el;
 >  6 |   try {
      |   ^^^^^
->  7 |     let value;
-     | ^^^^^^^^^^^^^^
->  8 |     try {
-     | ^^^^^^^^^^^^^^
->  9 |       value = identity(props.foo);
-     | ^^^^^^^^^^^^^^
-> 10 |     } catch {
-     | ^^^^^^^^^^^^^^
-> 11 |       el = <div value={value} />;
-     | ^^^^^^^^^^^^^^
-> 12 |     }
-     | ^^^^^^^^^^^^^^
-> 13 |   } finally {
-     | ^^^^^^^^^^^^^^
-> 14 |     console.log(el);
-     | ^^^^^^^^^^^^^^
+>    …
 > 15 |   }
      | ^^^^ (BuildHIR::lowerStatement) Handle TryStatement without a catch clause
   16 |   return el;

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-invalid-jsx-in-try-with-finally.expect.md

@@ -28,12 +28,7 @@ error.todo-invalid-jsx-in-try-with-finally.ts:4:2
    3 |   let el;
 >  4 |   try {
      |   ^^^^^
->  5 |     el = <div />;
-     | ^^^^^^^^^^^^^^^^^
->  6 |   } finally {
-     | ^^^^^^^^^^^^^^^^^
->  7 |     console.log(el);
-     | ^^^^^^^^^^^^^^^^^
+>    …
 >  8 |   }
      | ^^^^ (BuildHIR::lowerStatement) Handle TryStatement without a catch clause
    9 |   return el;