From fa35666535342ef173033deac4eaf6ff39fa4b63 Mon Sep 17 00:00:00 2001 From: Thomas Watson Date: Fri, 3 Jul 2026 11:35:49 +0200 Subject: [PATCH] Fix Live Debugger parenthesized arrow body instrumentation When an expression-body arrow has a parenthesized body, the transform strips the wrapper parens before injecting the block body. It did this with MagicString `remove()`, which clears the chunk's intro/outro. If the body was itself an instrumented function whose end abutted the closing paren (e.g. `(a) => ((b) => a + b)`), the inner function had already appended its instrumentation suffix to that offset, so `remove()` dropped it and emitted invalid JavaScript. Blank the wrapper parens with `update(..., '')` instead. It edits content only and preserves the inner function's appended suffix. Also make the wrapper-paren scanner comment-aware. It previously counted raw `(`/`)` characters, so a paren inside a comment (e.g. `() => ((x) /* ) */)`) was mistaken for a wrapper paren, misaligning removal and again producing invalid JavaScript. Only whitespace, comments, and wrapper parens can appear in that range, so any `/` must start a comment and can be skipped. Add regression coverage for parenthesized inner arrows (nested, JSX, and object bodies) and comment/regex bodies, plus previously untested async functions and class accessors. --- .../live-debugger/src/transform/index.test.ts | 185 ++++++++++++++++++ .../live-debugger/src/transform/index.ts | 64 ++++-- 2 files changed, 238 insertions(+), 11 deletions(-) diff --git a/packages/plugins/live-debugger/src/transform/index.test.ts b/packages/plugins/live-debugger/src/transform/index.test.ts index 8466e5ada..9a108296a 100644 --- a/packages/plugins/live-debugger/src/transform/index.test.ts +++ b/packages/plugins/live-debugger/src/transform/index.test.ts @@ -386,6 +386,48 @@ describe('transformCode', () => { expectedInstrumentedCount: 2, expectedTotalFunctions: 2, }, + { + description: 'parenthesized inner arrow abutting the closing paren', + code: 'const f = (a) => ((b) => a + b);', + namedOnly: false, + expectedInstrumentedCount: 2, + expectedTotalFunctions: 2, + }, + { + description: 'parenthesized inner arrow abutting the closing paren (namedOnly)', + code: 'const f = (a) => ((b) => a + b);', + namedOnly: true, + expectedInstrumentedCount: 1, + expectedTotalFunctions: 2, + }, + { + description: 'double-parenthesized inner arrow', + code: 'const f = (a) => (((b) => a + b));', + namedOnly: false, + expectedInstrumentedCount: 2, + expectedTotalFunctions: 2, + }, + { + description: 'nested parenthesized curried arrows', + code: 'const f = (a) => ((b) => ((c) => a + b + c));', + namedOnly: false, + expectedInstrumentedCount: 3, + expectedTotalFunctions: 3, + }, + { + description: 'parenthesized inner arrow returning JSX', + code: 'const f = (a) => ((b) =>
  • {a}{b}
    • );', + namedOnly: false, + expectedInstrumentedCount: 2, + expectedTotalFunctions: 2, + }, + { + description: 'parenthesized inner arrow returning a parenthesized object', + code: 'const f = (a) => ((b) => ({ sum: a + b }));', + namedOnly: false, + expectedInstrumentedCount: 2, + expectedTotalFunctions: 2, + }, ]; test.each(nestedSyntaxCases)( @@ -409,6 +451,149 @@ describe('transformCode', () => { ); }); + describe('parenthesized arrow bodies with comments', () => { + // The wrapper-paren scanner must skip comments; a stray paren inside a + // comment must not be mistaken for a wrapping paren, otherwise removal + // misaligns and produces invalid JavaScript. + const commentCases = [ + 'const f = () => (/* ( */ x);', + 'const f = () => (x /* ) */);', + 'const f = () => (/* ( */ (x));', + 'const f = () => ((x) /* ) */);', + 'const f = () => (\n // returns (something)\n value\n);', + 'const f = () => (/* le(ading */ (y) /* trai)ling */);', + // A regex/division `/` inside the body must not be treated as a + // comment start by the wrapper-paren scanner. + 'const f = () => (/[)(]/);', + 'const f = () => (/[)(]/g.test(x) ? a : b);', + ]; + + test.each(commentCases.map((code) => ({ code })))( + 'should produce valid output for $code', + ({ code }) => { + const result = transformCode({ ...BASE_OPTIONS, code }); + + expect(result.instrumentedCount).toBe(1); + expect(validateSyntax(result.code, '/src/utils.ts')).toBeNull(); + expect(result.code).toContain('$dd_return'); + expect(result.code).toContain('catch(e)'); + }, + ); + }); + + describe('async functions', () => { + const asyncCases = [ + { + description: 'async function declaration with await', + code: 'async function f(a) { return await g(a); }', + expectedInstrumentedCount: 1, + expectedTotalFunctions: 1, + }, + { + description: 'async arrow expression body', + code: 'const f = async (a) => await g(a);', + expectedInstrumentedCount: 1, + expectedTotalFunctions: 1, + }, + { + description: 'async arrow with unparenthesized param', + code: 'const f = async a => a;', + expectedInstrumentedCount: 1, + expectedTotalFunctions: 1, + }, + { + description: 'async arrow block body', + code: 'const f = async (a) => { return await g(a); };', + expectedInstrumentedCount: 1, + expectedTotalFunctions: 1, + }, + { + description: 'async arrow with parenthesized object body', + code: 'const f = async (a) => ({ v: await g(a) });', + expectedInstrumentedCount: 1, + expectedTotalFunctions: 1, + }, + { + description: 'async object method', + code: 'const o = { async m(a) { return await g(a); } };', + expectedInstrumentedCount: 1, + expectedTotalFunctions: 1, + }, + { + description: 'async class method', + code: 'class C { async m(a) { return await g(a); } }', + expectedInstrumentedCount: 1, + expectedTotalFunctions: 1, + }, + { + description: 'curried async arrow with parenthesized inner arrow', + code: 'const f = async (a) => ((b) => a + b);', + expectedInstrumentedCount: 2, + expectedTotalFunctions: 2, + }, + ]; + + test.each(asyncCases)( + 'should produce valid output for $description', + ({ code, expectedInstrumentedCount, expectedTotalFunctions }) => { + const result = transformCode({ ...BASE_OPTIONS, code }); + + expect(validateSyntax(result.code, '/src/utils.ts')).toBeNull(); + expect(result.instrumentedCount).toBe(expectedInstrumentedCount); + expect(result.totalFunctions).toBe(expectedTotalFunctions); + expect(result.code).toContain('$dd_probes'); + expect(result.code).toContain('catch(e)'); + }, + ); + }); + + describe('accessors and static members', () => { + const accessorCases = [ + { + description: 'class getter', + code: 'class C { get x() { return this._x; } }', + expectedInstrumentedCount: 1, + expectedTotalFunctions: 1, + }, + { + description: 'class setter', + code: 'class C { set x(v) { this._x = v; } }', + expectedInstrumentedCount: 1, + expectedTotalFunctions: 1, + }, + { + description: 'object getter and setter', + code: 'const o = { get x() { return 1; }, set x(v) { this._x = v; } };', + expectedInstrumentedCount: 2, + expectedTotalFunctions: 2, + }, + { + description: 'static method', + code: 'class C { static m(a) { return a; } }', + expectedInstrumentedCount: 1, + expectedTotalFunctions: 1, + }, + { + description: 'static block alongside a method', + code: 'class C { static { init(); } m() { return 1; } }', + expectedInstrumentedCount: 1, + expectedTotalFunctions: 1, + }, + ]; + + test.each(accessorCases)( + 'should produce valid output for $description', + ({ code, expectedInstrumentedCount, expectedTotalFunctions }) => { + const result = transformCode({ ...BASE_OPTIONS, code }); + + expect(validateSyntax(result.code, '/src/utils.ts')).toBeNull(); + expect(result.instrumentedCount).toBe(expectedInstrumentedCount); + expect(result.totalFunctions).toBe(expectedTotalFunctions); + expect(result.code).toContain('catch(e)'); + }, + ); + }); + describe('skipping', () => { it('should skip generators', () => { const result = transformCode({ diff --git a/packages/plugins/live-debugger/src/transform/index.ts b/packages/plugins/live-debugger/src/transform/index.ts index df69f69ee..5abc0a099 100644 --- a/packages/plugins/live-debugger/src/transform/index.ts +++ b/packages/plugins/live-debugger/src/transform/index.ts @@ -486,6 +486,13 @@ function injectInstrumentation(s: MagicStringType, code: string, target: Functio // otherwise the output would be => ({block_code}) which is a syntax error. // Babel stores nested bodies like `((1))` as body `1`, so every wrapper // paren around the body range must be removed before injecting a block. + // + // Blank the paren with `update(..., '')` rather than `remove()`. When the + // body is itself an instrumented function whose end abuts the closing + // paren (e.g. `(a) => ((b) => a + b)`), that inner function has already + // appended its instrumentation suffix to the same offset. `remove()` + // clears the chunk's intro/outro and would drop that suffix, producing + // invalid JavaScript; `update()` edits the content only and preserves it. if (bodyParenStart != null) { const openingParens = getLeadingArrowBodyParens(code, bodyParenStart, bodyStart); const closingParens = getTrailingArrowBodyParens(code, bodyEnd, functionEnd); @@ -494,8 +501,8 @@ function injectInstrumentation(s: MagicStringType, code: string, target: Functio for (let i = 0; i < parenCount; i++) { const openingParen = openingParens[i]; const closingParen = closingParens[i]; - s.remove(openingParen, openingParen + 1); - s.remove(closingParen, closingParen + 1); + s.update(openingParen, openingParen + 1, ''); + s.update(closingParen, closingParen + 1, ''); } } @@ -812,21 +819,56 @@ function getReturnCaptureArgs( } function getLeadingArrowBodyParens(code: string, start: number, end: number): number[] { - const parens: number[] = []; - for (let i = start; i < end; i++) { - if (code[i] === '(') { - parens.push(i); - } - } - return parens; + return collectArrowWrapperParens(code, start, end, '('); } function getTrailingArrowBodyParens(code: string, start: number, end: number): number[] { + return collectArrowWrapperParens(code, start, end, ')'); +} + +/** + * Collect the positions of wrapper parens around an arrow expression body. + * + * Only whitespace, comments, and the wrapping parens themselves can appear + * between the outermost paren and the body (leading) or between the body and + * the arrow's end (trailing). Comments are skipped so that parens inside a + * comment are not mistaken for wrapper parens, which would otherwise misalign + * removal and emit invalid JavaScript. + * + * Any `/` in this range necessarily starts a comment (division/regex are not + * grammatically valid here), so no string/regex handling is required. + */ +function collectArrowWrapperParens( + code: string, + start: number, + end: number, + paren: '(' | ')', +): number[] { const parens: number[] = []; - for (let i = end - 1; i >= start; i--) { - if (code[i] === ')') { + let i = start; + while (i < end) { + const char = code[i]; + if (char === '/') { + const next = code[i + 1]; + if (next === '/') { + i += 2; + while (i < end && code[i] !== '\n') { + i++; + } + continue; + } + if (next === '*') { + i += 2; + while (i < end && !(code[i] === '*' && code[i + 1] === '/')) { + i++; + } + i += 2; + continue; + } + } else if (char === paren) { parens.push(i); } + i++; } return parens; }