Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,7 @@ export const swc: PresetProperty<'swc'> = (config: Record<string, any>): Record<
safari: 15,
firefox: 91,
},
// Transpiles the broken syntax to the closest non-broken modern syntax.
// E.g. it won't transpile parameter destructuring in Safari
// which would break how we detect if the mount context property is used in the play function.
bugfixes: config?.env?.bugfixes ?? true,
bugfixes: config?.env?.bugfixes ?? false,
},
};
};
Expand Down
1 change: 0 additions & 1 deletion code/core/src/core-server/presets/common-preset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,6 @@ export const babel = async (_: unknown, options: Options) => {
[
'@babel/preset-env',
{
bugfixes: true,
targets: {
// This is the same browser supports that we use to bundle our manager and preview code.
chrome: 100,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,20 +32,112 @@ const LongDefinition = {
},
};

test('Detect destructure', () => {
const MethodProperty = {
async play({
mount,
veryLongDefinitionnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnn,
over,
multiple,
lines,
}: any) {
await mount();
},
};

const TranspiledDefinition = {
play: async (context: any) => {
const {
mount,
veryLongTranspiledDefinitionnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnn,
over,
multiple,
lines,
} = context;
await mount();
},
};

const LateDestructuring = {
play: async (a: any) => {
console.log(a);
const {
mount,
veryLongTranspiledDefinitionnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnn,
over,
multiple,
lines,
} = a;
await mount();
},
};
Comment thread
ndelangen marked this conversation as resolved.

const WithComment = {
play: async (context: any) => {
const {
// a comment
mount,
} = context;
await mount();
},
};

const WithTrailingComment = {
play: async (context: any) => {
const {
mount, // a comment
} = context;
await mount();
},
};

const WithMultipleComments = {
play: async (context: any) => {
const {
mount, // a comment
// another comment
} = context;
await mount();
},
};

const WithBlockComments = {
play: async (context: any) => {
const { mount /* a comment */ } = context;
/* another comment */
await mount();
/* third comment */
},
};

const testingScope = {
mount: async (m: any) => {
return 'testingScope.mount';
},
};

const IncorrectMount = {
play: async (context: any) => {
const { mount } = testingScope;
const { mount: sbMount } = context;
mount(await sbMount());
},
};

test('Detect basic destructuring', () => {
expect(getUsedProps(StoryWithContext.play)).toMatchInlineSnapshot(`[]`);
expect(getUsedProps(StoryWitCanvasElement.play)).toMatchInlineSnapshot(`
[
"canvasElement",
]
`);

expect(getUsedProps(MountStory.play)).toMatchInlineSnapshot(`
[
"mount",
]
`);
});

test('Detect multiline destructuring', () => {
expect(getUsedProps(LongDefinition.play)).toMatchInlineSnapshot(`
[
"mount",
Expand All @@ -55,4 +147,63 @@ test('Detect destructure', () => {
"lines",
]
`);
expect(getUsedProps(MethodProperty.play)).toMatchInlineSnapshot(`
[
"mount",
"veryLongDefinitionnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnn",
"over",
"multiple",
"lines",
]
`);
});

test('Detect transpiled destructuring', () => {
expect(getUsedProps(TranspiledDefinition.play)).toMatchInlineSnapshot(`
[
"mount",
"veryLongTranspiledDefinitionnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnn",
"over",
"multiple",
"lines",
]
`);

expect(getUsedProps(LateDestructuring.play)).toMatchInlineSnapshot(`[]`);
});

test('Detect with comment', () => {
expect(getUsedProps(WithComment.play)).toMatchInlineSnapshot(`
[
"mount",
]
`);
});

test('Detect with trailing comment', () => {
expect(getUsedProps(WithTrailingComment.play)).toMatchInlineSnapshot(`
[
"mount",
]
`);
});

test('Detect with multiple comments', () => {
expect(getUsedProps(WithMultipleComments.play)).toMatchInlineSnapshot(`
[
"mount",
]
`);
});

test('Detect with block comments', () => {
expect(getUsedProps(WithBlockComments.play)).toMatchInlineSnapshot(`
[
"mount",
]
`);
});

test('Detect incorrect mount', () => {
expect(getUsedProps(IncorrectMount.play)).toMatchInlineSnapshot(`[]`);
});
64 changes: 51 additions & 13 deletions code/core/src/preview-api/modules/preview-web/render/mount-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,32 +4,70 @@ export function mountDestructured(playFunction?: (...args: any[]) => any): boole
return playFunction != null && getUsedProps(playFunction).includes('mount');
}

export function getUsedProps(fn: (...args: any[]) => any) {
const match = fn.toString().match(/[^(]*\(([^)]*)/);

if (!match) {
/**
* Extracts a list of properties destructured from the argument of a play function, either inline or
* as the first statement in the body of the function.
*
* @param fn - The function to extract the properties from.
* @returns An array of property names.
*/
export function getUsedProps(fn: (...args: unknown[]) => unknown) {
const [, args, body] = fn.toString().match(/[^(]*\(([^)]+)\)(?:.*{([^]+)})?/) || [];
if (!args) {
return [];
}

const args = splitByComma(match[1]);

if (!args.length) {
const [firstArg] = splitByComma(args);
if (!firstArg) {
return [];
}

const first = args[0];
const [, destructuredProps] = firstArg.match(/^{([^]+)}$/) || [];
if (destructuredProps) {
return splitByComma(stripComments(destructuredProps)).map((prop) =>
prop.replace(/:.*|=.*/g, '').trim()
);
}

if (!(first.startsWith('{') && first.endsWith('}'))) {
if (!firstArg.match(/^[a-z_$][0-9a-z_$]*$/i)) {
return [];
}

const props = splitByComma(first.slice(1, -1).replace(/\s/g, '')).map((prop) => {
return prop.replace(/:.*|=.*/g, '');
});
const escapedArg = firstArg.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
const [, destructuredArg] =
body?.trim()?.match(new RegExp(`^(?:const|let|var)\\s*{([^}]+)}\\s*=\\s*${escapedArg};`)) || [];
if (destructuredArg) {
return splitByComma(stripComments(destructuredArg)).map((prop) =>
prop.replace(/:.*|=.*/g, '').trim()
);
}
Comment thread
ghengeveld marked this conversation as resolved.
Comment thread
ghengeveld marked this conversation as resolved.

return [];
}
Comment on lines +14 to +46
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Fix Biome regex errors + make body-capture robust for single-line play functions

  • [^]+ trips Biome (noEmptyCharacterClassInRegex) at Line 15 and Line 25.
  • Current body capture can fail when the entire function is on one line and contains additional {} (e.g., const { mount } = context;).
Proposed diff
-export function getUsedProps(fn: (...args: unknown[]) => unknown) {
-  const [, args, body] = fn.toString().match(/[^(]*\(([^)]+)\)(?:.*{([^]+)})?/) || [];
+export function getUsedProps(fn: (...args: unknown[]) => unknown): string[] {
+  const src = fn.toString();
+  const [, args] = src.match(/^[^(]*\(([^)]*)\)/) || [];
+  const [, body] = src.match(/^[^(]*\([^)]*\)[^{]*{([\s\S]*)}\s*$/) || [];
   if (!args) {
     return [];
   }

   const [firstArg] = splitByComma(args);
   if (!firstArg) {
     return [];
   }

-  const [, destructuredProps] = firstArg.match(/^{([^]+)}$/) || [];
+  const [, destructuredProps] = firstArg.match(/^{([\s\S]*)}$/) || [];
   if (destructuredProps) {
     return splitByComma(stripComments(destructuredProps)).map((prop) =>
       prop.replace(/:.*|=.*/g, '').trim()
     );
   }

   if (!firstArg.match(/^[a-z_$][0-9a-z_$]*$/i)) {
     return [];
   }

   const escapedArg = firstArg.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
   const [, destructuredArg] =
-    body?.trim()?.match(new RegExp(`^(?:const|let|var)\\s*{([^}]+)}\\s*=\\s*${escapedArg};`)) || [];
+    body?.trim()?.match(
+      new RegExp(`^(?:const|let|var)\\s*{([\\s\\S]*?)}\\s*=\\s*${escapedArg}\\s*;?`)
+    ) || [];
   if (destructuredArg) {
     return splitByComma(stripComments(destructuredArg)).map((prop) =>
       prop.replace(/:.*|=.*/g, '').trim()
     );
   }

   return [];
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export function getUsedProps(fn: (...args: unknown[]) => unknown) {
const [, args, body] = fn.toString().match(/[^(]*\(([^)]+)\)(?:.*{([^]+)})?/) || [];
if (!args) {
return [];
}
const args = splitByComma(match[1]);
if (!args.length) {
const [firstArg] = splitByComma(args);
if (!firstArg) {
return [];
}
const first = args[0];
const [, destructuredProps] = firstArg.match(/^{([^]+)}$/) || [];
if (destructuredProps) {
return splitByComma(stripComments(destructuredProps)).map((prop) =>
prop.replace(/:.*|=.*/g, '').trim()
);
}
if (!(first.startsWith('{') && first.endsWith('}'))) {
if (!firstArg.match(/^[a-z_$][0-9a-z_$]*$/i)) {
return [];
}
const props = splitByComma(first.slice(1, -1).replace(/\s/g, '')).map((prop) => {
return prop.replace(/:.*|=.*/g, '');
});
const escapedArg = firstArg.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
const [, destructuredArg] =
body?.trim()?.match(new RegExp(`^(?:const|let|var)\\s*{([^}]+)}\\s*=\\s*${escapedArg};`)) || [];
if (destructuredArg) {
return splitByComma(stripComments(destructuredArg)).map((prop) =>
prop.replace(/:.*|=.*/g, '').trim()
);
}
return [];
}
export function getUsedProps(fn: (...args: unknown[]) => unknown): string[] {
const src = fn.toString();
const [, args] = src.match(/^[^(]*\(([^)]*)\)/) || [];
const [, body] = src.match(/^[^(]*\([^)]*\)[^{]*{([\s\S]*)}\s*$/) || [];
if (!args) {
return [];
}
const [firstArg] = splitByComma(args);
if (!firstArg) {
return [];
}
const [, destructuredProps] = firstArg.match(/^{([\s\S]*)}$/) || [];
if (destructuredProps) {
return splitByComma(stripComments(destructuredProps)).map((prop) =>
prop.replace(/:.*|=.*/g, '').trim()
);
}
if (!firstArg.match(/^[a-z_$][0-9a-z_$]*$/i)) {
return [];
}
const escapedArg = firstArg.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
const [, destructuredArg] =
body?.trim()?.match(
new RegExp(`^(?:const|let|var)\\s*{([\\s\\S]*?)}\\s*=\\s*${escapedArg}\\s*;?`)
) || [];
if (destructuredArg) {
return splitByComma(stripComments(destructuredArg)).map((prop) =>
prop.replace(/:.*|=.*/g, '').trim()
);
}
return [];
}
🧰 Tools
🪛 ast-grep (0.40.4)

[warning] 37-37: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(^(?:const|let|var)\\s*{([^}]+)}\\s*=\\s*${escapedArg};)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html

(regexp-from-variable)

🪛 Biome (2.1.2)

[error] 15-15: The regular expression includes this negated empty character class.

Negated empty character classes match anything.
If you want to match against [, escape it [.
Otherwise, remove the character class or fill it.

(lint/correctness/noEmptyCharacterClassInRegex)


[error] 25-25: The regular expression includes this negated empty character class.

Negated empty character classes match anything.
If you want to match against [, escape it [.
Otherwise, remove the character class or fill it.

(lint/correctness/noEmptyCharacterClassInRegex)


return props;
/**
* Strips JavaScript comments from a string.
*
* @param s - The string to strip comments from.
* @returns The string with comments removed.
*/
function stripComments(s: string): string {
// Remove single-line comments (// ...)
s = s.replace(/\/\/.*$/gm, '');
// Remove multi-line comments (/* ... */)
s = s.replace(/\/\*[\s\S]*?\*\//g, '');
return s;
}

/**
* Splits a string by top-level commas, ignoring commas nested within curly or square brackets.
*
* This is useful for parsing function argument lists or destructured object patterns where elements
* inside nested structures (like { a, b: [x, y], c }) should not be split.
*
* @param s - The string to split.
* @returns An array of substrings split by top-level commas.
*/
function splitByComma(s: string) {
const result = [];
const stack = [];
Expand Down
1 change: 0 additions & 1 deletion code/frameworks/nextjs/src/babel/preset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ export default (api: any, options: NextBabelPresetOptions = {}): BabelPreset =>
// In production/development this option is set to `false` so that webpack can handle import/export with tree-shaking
modules: 'auto',
exclude: ['transform-typeof-symbol'],
bugfixes: true,
targets: {
chrome: 100,
safari: 15,
Expand Down
1 change: 0 additions & 1 deletion code/frameworks/nextjs/src/preset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,6 @@ export const babel: PresetProperty<'babel'> = async (baseConfig: TransformOption
[
'next/dist/compiled/babel/preset-env',
{
bugfixes: true,
targets: {
chrome: 100,
safari: 15,
Expand Down
4 changes: 0 additions & 4 deletions code/frameworks/nextjs/src/swc/next-swc-loader-patch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,6 @@ async function loaderTransform(this: any, parentTrace: any, source?: string, inp
// modules.
sourceFileName: filename,
};
// Transpiles the broken syntax to the closest non-broken modern syntax.
// E.g. it won't transpile parameter destructuring in Safari
// which would break how we detect if the mount context property is used in the play function.
programmaticOptions.env.bugfixes = true;

if (!programmaticOptions.inputSourceMap) {
delete programmaticOptions.inputSourceMap;
Expand Down