From c4201b608f2147ec1d2e1774173dd6fdbc7fede0 Mon Sep 17 00:00:00 2001 From: Emilia <100027345+sovietspaceship@users.noreply.github.com> Date: Tue, 6 Oct 2026 09:59:47 +0100 Subject: [PATCH] revert: Keep RegexEngine's exec/matchAll/split on the native array types RegexExecArray typed non-participating capture groups as string | undefined instead of RegExpExecArray's string, which is a breaking compile-time change for any external TypeScript consumer of the published n8n-workflow types. Revert the public interfaces back to the native shapes; a pattern engine selection is more of a product decision than a type-soundness one, and the real fix (Notion's explicit undefined check) doesn't depend on the stricter type to work. Co-Authored-By: Claude Sonnet 5 --- .../editor-ui/src/app/utils/safeRegex.ts | 22 ++++++------ packages/workflow/src/safe-regex.ts | 36 ++++++++----------- packages/workflow/test/safe-regex.test.ts | 12 ++++--- 3 files changed, 33 insertions(+), 37 deletions(-) diff --git a/packages/frontend/editor-ui/src/app/utils/safeRegex.ts b/packages/frontend/editor-ui/src/app/utils/safeRegex.ts index 779ff2202ab..c1cb91b98a9 100644 --- a/packages/frontend/editor-ui/src/app/utils/safeRegex.ts +++ b/packages/frontend/editor-ui/src/app/utils/safeRegex.ts @@ -1,4 +1,4 @@ -import type { RegexEngineAsync, RegexExecArray } from 'n8n-workflow'; +import type { RegexEngineAsync } from 'n8n-workflow'; const REGEX_TIMEOUT_MS = 250; const REGEX_TIMEOUT_ERROR_MESSAGE = 'Regular expression execution timed out'; @@ -22,7 +22,7 @@ type RegexResponse = { let requestId = 0; -function isRegExpMatch(value: unknown): value is RegexExecArray { +function isRegExpMatch(value: unknown): value is RegExpMatchArray { return ( Array.isArray(value) && typeof Reflect.get(value, 'index') === 'number' && @@ -30,22 +30,20 @@ function isRegExpMatch(value: unknown): value is RegexExecArray { ); } -function isRegExpMatchOrNull(value: unknown): value is RegexExecArray | null { +function isRegExpMatchOrNull(value: unknown): value is RegExpExecArray | null { return value === null || isRegExpMatch(value); } -function isRegExpMatchArray(value: unknown): value is RegexExecArray[] { +function isRegExpMatchArray(value: unknown): value is RegExpMatchArray[] { return Array.isArray(value) && value.every(isRegExpMatch); } -function isSplitResultArray(value: unknown): value is Array { - return ( - Array.isArray(value) && value.every((item) => item === undefined || typeof item === 'string') - ); +function isStringArray(value: unknown): value is string[] { + return Array.isArray(value) && value.every((item) => typeof item === 'string'); } export class WorkerRegexEngine implements RegexEngineAsync { - async exec(pattern: string, input: string, flags?: string): Promise { + async exec(pattern: string, input: string, flags?: string): Promise { const result = await this.execute('exec', pattern, input, flags); if (!isRegExpMatchOrNull(result)) throw new Error('Unexpected regex execution result'); return result; @@ -68,15 +66,15 @@ export class WorkerRegexEngine implements RegexEngineAsync { return result; } - async matchAll(pattern: string, input: string, flags?: string): Promise { + async matchAll(pattern: string, input: string, flags?: string): Promise { const result = await this.execute('matchAll', pattern, input, flags); if (!isRegExpMatchArray(result)) throw new Error('Unexpected regex execution result'); return result; } - async split(pattern: string, input: string, flags?: string): Promise> { + async split(pattern: string, input: string, flags?: string): Promise { const result = await this.execute('split', pattern, input, flags); - if (!isSplitResultArray(result)) throw new Error('Unexpected regex execution result'); + if (!isStringArray(result)) throw new Error('Unexpected regex execution result'); return result; } diff --git a/packages/workflow/src/safe-regex.ts b/packages/workflow/src/safe-regex.ts index dbe5562647a..06978bc4df2 100644 --- a/packages/workflow/src/safe-regex.ts +++ b/packages/workflow/src/safe-regex.ts @@ -3,28 +3,22 @@ import * as LoggerProxy from './logger-proxy'; const REGEX_TIMEOUT_MS = 250; const REGEX_TIMEOUT_ERROR_MESSAGE = 'Regular expression execution timed out'; -/** - * A match result. A non-participating capture group is `undefined`, which the built-in - * engine also produces at runtime but `RegExpExecArray` claims cannot happen. - */ -export type RegexExecArray = Array & { - // A successful match always sets group 0; only capture groups can be unset. - 0: string; - index?: number; - input?: string; - groups?: Record; -}; - +// `RegExpExecArray`/`RegExpMatchArray` type every capture group as `string`, but a +// non-participating group is `undefined` at runtime. Kept here regardless, matching the +// native types on purpose: these interfaces are public (re-exported from n8n-workflow), +// and the accurate `string | undefined` element type is a breaking change for any +// external consumer pinned to the native shapes. Code that dereferences a capture group +// must still guard against `undefined` itself; the type won't catch a missing guard. export interface RegexEngine { - exec(pattern: string, input: string, flags?: string): RegexExecArray | null; + exec(pattern: string, input: string, flags?: string): RegExpExecArray | null; test(pattern: string, input: string, flags?: string): boolean; replace(pattern: string, input: string, flags: string | undefined, replacement: string): string; - matchAll(pattern: string, input: string, flags?: string): RegexExecArray[]; - split(pattern: string, input: string, flags?: string): Array; + matchAll(pattern: string, input: string, flags?: string): RegExpMatchArray[]; + split(pattern: string, input: string, flags?: string): string[]; } export interface RegexEngineAsync { - exec(pattern: string, input: string, flags?: string): Promise; + exec(pattern: string, input: string, flags?: string): Promise; test(pattern: string, input: string, flags?: string): Promise; replace( pattern: string, @@ -32,8 +26,8 @@ export interface RegexEngineAsync { flags: string | undefined, replacement: string, ): Promise; - matchAll(pattern: string, input: string, flags?: string): Promise; - split(pattern: string, input: string, flags?: string): Promise>; + matchAll(pattern: string, input: string, flags?: string): Promise; + split(pattern: string, input: string, flags?: string): Promise; } export type RegexLiteral = { @@ -154,7 +148,7 @@ function nodeVmEngine(vm: VmModule): RegexEngine { context.flags = flags ?? ''; context.input = input; context.result = undefined; - return run(scripts.exec); + return run(scripts.exec); }, test(pattern, input, flags) { context.pattern = pattern; @@ -176,14 +170,14 @@ function nodeVmEngine(vm: VmModule): RegexEngine { context.flags = globalFlag(flags); context.input = input; context.result = undefined; - return run(scripts.matchAll); + return run(scripts.matchAll); }, split(pattern, input, flags) { context.pattern = pattern; context.flags = flags ?? ''; context.input = input; context.result = undefined; - return run>(scripts.split); + return run(scripts.split); }, }; } diff --git a/packages/workflow/test/safe-regex.test.ts b/packages/workflow/test/safe-regex.test.ts index 96f55d4810e..cfdc60e10ca 100644 --- a/packages/workflow/test/safe-regex.test.ts +++ b/packages/workflow/test/safe-regex.test.ts @@ -49,20 +49,24 @@ describe('safeUserRegex', () => { }); const engine = { - exec: vi.fn(() => Object.assign(['match'], { 0: 'match' })), + exec: vi.fn(() => Object.assign(['match'], { 0: 'match', index: 0, input: 'input' })), test: vi.fn(() => true), replace: vi.fn(() => 'replaced'), - matchAll: vi.fn(() => [Object.assign(['match'], { 0: 'match' })]), + matchAll: vi.fn(() => [Object.assign(['match'], { 0: 'match', index: 0, input: 'input' })]), split: vi.fn(() => ['a', 'b']), }; it('delegates operations to the configured user engine', () => { setUserRegexEngine(engine); - expect(safeUserRegex.exec('source', 'input')).toEqual(['match']); + expect(safeUserRegex.exec('source', 'input')).toEqual( + Object.assign(['match'], { index: 0, input: 'input' }), + ); expect(safeUserRegex.test('source', 'input')).toBe(true); expect(safeUserRegex.replace('source', 'input', 'g', 'replacement')).toBe('replaced'); - expect(safeUserRegex.matchAll('source', 'input')).toEqual([['match']]); + expect(safeUserRegex.matchAll('source', 'input')).toEqual([ + Object.assign(['match'], { index: 0, input: 'input' }), + ]); expect(safeUserRegex.split('source', 'input')).toEqual(['a', 'b']); });