From 408b38ef7304faf022d2a37110c57efce12c6bad Mon Sep 17 00:00:00 2001 From: Joseph Savona <6425824+josephsavona@users.noreply.github.com> Date: Thu, 23 Oct 2025 11:30:28 -0700 Subject: [PATCH] [compiler] Improve display of errors on multi-line expressions (#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. --- .../src/CompilerError.ts | 41 ++++++++++++++++++- ...es-memoizes-with-captures-values.expect.md | 24 +---------- ...ional-nonoptional-property-chain.expect.md | 15 +------ .../bailout-retry/error.todo-syntax.expect.md | 13 +----- 4 files changed, 43 insertions(+), 50 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts b/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts index 3c131ea653..18bcd7791d 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts +++ b/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 = 10; +/* + * 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 = 5; + 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) + '…', + ...lines.slice( + -(CODEFRAME_LINES_BELOW + CODEFRAME_ABBREVIATED_SOURCE_LINES), + ), + ].join('\n'); } function printErrorSummary(category: ErrorCategory, message: string): string { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-uncalled-function-capturing-mutable-values-memoizes-with-captures-values.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-uncalled-function-capturing-mutable-values-memoizes-with-captures-values.expect.md index 8592ae65e4..250114fdfb 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-uncalled-function-capturing-mutable-values-memoizes-with-captures-values.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-uncalled-function-capturing-mutable-values-memoizes-with-captures-values.expect.md @@ -60,29 +60,7 @@ This argument is a function which may reassign or mutate `cache` after render, w > 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(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -> 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 | }; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-preserve-memo-deps-mixed-optional-nonoptional-property-chain.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-preserve-memo-deps-mixed-optional-nonoptional-property-chain.expect.md index e9772e6799..45e3d365a8 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-preserve-memo-deps-mixed-optional-nonoptional-property-chain.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-preserve-memo-deps-mixed-optional-nonoptional-property-chain.expect.md @@ -64,20 +64,7 @@ error.todo-preserve-memo-deps-mixed-optional-nonoptional-property-chain.ts:7:25 > 8 | return identity({ | ^^^^^^^^^^^^^^^^^^^^^ > 9 | callback: () => { - | ^^^^^^^^^^^^^^^^^^^^^ -> 10 | // This is a bug in our dependency inference: we stop capturing dependencies - | ^^^^^^^^^^^^^^^^^^^^^ -> 11 | // after x.a.b?.c. But what this dependency is telling us is that if `x.a.b` - | ^^^^^^^^^^^^^^^^^^^^^ -> 12 | // was non-nullish, then we can access `.c.d?.e`. Thus we should take the - | ^^^^^^^^^^^^^^^^^^^^^ -> 13 | // full property chain, exactly as-is with optionals/non-optionals, as a - | ^^^^^^^^^^^^^^^^^^^^^ -> 14 | // dependency - | ^^^^^^^^^^^^^^^^^^^^^ -> 15 | return identity(x.a.b?.c.d?.e); - | ^^^^^^^^^^^^^^^^^^^^^ -> 16 | }, + … | ^^^^^^^^^^^^^^^^^^^^^ > 17 | }); | ^^^^^^^^^^^^^^^^^^^^^ diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.todo-syntax.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.todo-syntax.expect.md index 38d10ee0d1..00af7ec6ad 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.todo-syntax.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.todo-syntax.expect.md @@ -46,18 +46,7 @@ error.todo-syntax.ts:11:2 > 12 | () => { | ^^^^^^^^^^^ > 13 | try { - | ^^^^^^^^^^^ -> 14 | console.log(prop1); - | ^^^^^^^^^^^ -> 15 | } finally { - | ^^^^^^^^^^^ -> 16 | console.log('exiting'); - | ^^^^^^^^^^^ -> 17 | } - | ^^^^^^^^^^^ -> 18 | }, - | ^^^^^^^^^^^ -> 19 | [prop1], + … | ^^^^^^^^^^^ > 20 | AUTODEPS | ^^^^^^^^^^^