mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-17 05:29:44 +02:00
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 <noreply@anthropic.com>
This commit is contained in:
parent
dafac78dd0
commit
c4201b608f
|
|
@ -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<string | undefined> {
|
||||
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<RegexExecArray | null> {
|
||||
async exec(pattern: string, input: string, flags?: string): Promise<RegExpExecArray | null> {
|
||||
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<RegexExecArray[]> {
|
||||
async matchAll(pattern: string, input: string, flags?: string): Promise<RegExpMatchArray[]> {
|
||||
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<Array<string | undefined>> {
|
||||
async split(pattern: string, input: string, flags?: string): Promise<string[]> {
|
||||
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;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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<string | undefined> & {
|
||||
// A successful match always sets group 0; only capture groups can be unset.
|
||||
0: string;
|
||||
index?: number;
|
||||
input?: string;
|
||||
groups?: Record<string, string | undefined>;
|
||||
};
|
||||
|
||||
// `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<string | undefined>;
|
||||
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<RegexExecArray | null>;
|
||||
exec(pattern: string, input: string, flags?: string): Promise<RegExpExecArray | null>;
|
||||
test(pattern: string, input: string, flags?: string): Promise<boolean>;
|
||||
replace(
|
||||
pattern: string,
|
||||
|
|
@ -32,8 +26,8 @@ export interface RegexEngineAsync {
|
|||
flags: string | undefined,
|
||||
replacement: string,
|
||||
): Promise<string>;
|
||||
matchAll(pattern: string, input: string, flags?: string): Promise<RegexExecArray[]>;
|
||||
split(pattern: string, input: string, flags?: string): Promise<Array<string | undefined>>;
|
||||
matchAll(pattern: string, input: string, flags?: string): Promise<RegExpMatchArray[]>;
|
||||
split(pattern: string, input: string, flags?: string): Promise<string[]>;
|
||||
}
|
||||
|
||||
export type RegexLiteral = {
|
||||
|
|
@ -154,7 +148,7 @@ function nodeVmEngine(vm: VmModule): RegexEngine {
|
|||
context.flags = flags ?? '';
|
||||
context.input = input;
|
||||
context.result = undefined;
|
||||
return run<RegexExecArray | null>(scripts.exec);
|
||||
return run<RegExpExecArray | null>(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<RegexExecArray[]>(scripts.matchAll);
|
||||
return run<RegExpMatchArray[]>(scripts.matchAll);
|
||||
},
|
||||
split(pattern, input, flags) {
|
||||
context.pattern = pattern;
|
||||
context.flags = flags ?? '';
|
||||
context.input = input;
|
||||
context.result = undefined;
|
||||
return run<Array<string | undefined>>(scripts.split);
|
||||
return run<string[]>(scripts.split);
|
||||
},
|
||||
};
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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']);
|
||||
});
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user