Fix #5: handle chained-call strip edge cases safely by sam-parsons · Pull Request #12 · sam-parsons/rolldown-plugin-strip

sam-parsons · 2026-04-26T14:12:09Z

Summary

Replaced regex-only function stripping with a line-based pass that finds the matching closing ) for the configured call.
Removes a line only when the call is the whole statement (optional whitespace and trailing ; after the call).
Adds chainedCalls option: skip (default), warn (console.warn), or error (throws from transform) when a line matches a configured call but has trailing code after the call (chains like .then, or other expressions).

Issues

Fixes Issue E: Handle chained-call strip edge cases safely #5

How to test

npm run check
npm run build

Notes

Line-based analysis matches prior single-line behavior; multi-line call statements are out of scope for this pass.
Warn path uses console.warn today; wiring to Rolldown/Rollup this.warn can follow if we extend the transform signature.

Learning notes

Fail-safe default for destructive transforms

What it is
When a build transform can change runtime behavior, the default policy should be the least surprising: do nothing ambiguous rather than emit broken code.

Where it appears in this PR
src/index.ts — chainedCalls defaults to skip, so ambiguous lines are left unchanged unless the user opts into warn or error.

Why it was the right call here
The alternative default is aggressive stripping (e.g. turning fn().chain into invalid or throwing code). Default skip matches the sharp-edge story from @rollup/plugin-strip and keeps production builds safe until the user explicitly chooses stricter behavior.

Optional mini snippet (when helpful)

// Safe whole-statement strip:
console.log("x");

// Not stripped with default chainedCalls: "skip"
console.log("x").then(() => {});

If you want to go deeper
Search: "defensive defaults" and "parse vs replace for code transforms".

Add chainedCalls policy (skip, warn, error) and line-aware call stripping so member/call chains after a target call are not removed in ways that would break runtime.

sam-parsons · 2026-04-26T14:59:43Z

+}

-  return output;
+function stripConfiguredCallExpressions(


needs jsdoc

Added in 4cbeceb — I added JSDoc for stripConfiguredCallExpressions and analyzeCallExpressionLine to describe responsibilities and behavior.

sam-parsons · 2026-04-26T15:00:18Z

+  chainedCalls: ChainedCallsPolicy,
+  moduleId: string
+): string {
+  const lines = code.split(/\r?\n/);


please leave high level comments through the body of stripConfiguredCallExpressions, explaining meaning of parts of the code

Done in 4cbeceb — I added high-level inline comments through stripConfiguredCallExpressions explaining line-pass processing, per-function matching, safe-removal path, chained policy path, and line retention.

sam-parsons · 2026-04-26T15:01:12Z

-    const escaped = escapeRegExp(fn);
-    const pattern = new RegExp(`^[ \\t]*${escaped}\\s*\\([^\\n\\r;]*\\)\\s*;?[ \\t]*\\r?\\n?`, "gm");
-    output = output.replace(pattern, "");
+function analyzeCallExpressionLine(line: string, fn: string): { matched: boolean; safeRemove: boolean } {


jsdoc for this and high level comments in the function body

Added in 4cbeceb — analyzeCallExpressionLine now has JSDoc and I added body-level high-level comments for both analysis and stripping flow.

there is no high-level comments in this function body

Addressed in 385680c — I added short step-by-step comments inside analyzeCallExpressionLine (indent check, call shape, paren walk, tail check) so the body reads at a glance.

Fixed in 385680c — added step-by-step comments inside analyzeCallExpressionLine (indent, call shape, paren walk, tail/safe-remove check).

sam-parsons · 2026-04-26T15:01:42Z

-    const pattern = new RegExp(`^[ \\t]*${escaped}\\s*\\([^\\n\\r;]*\\)\\s*;?[ \\t]*\\r?\\n?`, "gm");
-    output = output.replace(pattern, "");
+function analyzeCallExpressionLine(line: string, fn: string): { matched: boolean; safeRemove: boolean } {
+  const wsMatch = /^[ \t]*/.exec(line);


not sure what the ws prefix on these variables mean

Good call — updated in 4cbeceb. I renamed wsMatch / wsLen to leadingWhitespaceMatch / leadingWhitespaceLength for clarity.

sam-parsons · 2026-04-26T15:03:25Z

+function findClosingParen(line: string, openParenIndex: number): number {
+  let depth = 0;
+  for (let i = openParenIndex; i < line.length; i += 1) {
+    const c = line[i];


more semantic variable names

Updated in 385680c — renamed locals in analyzeCallExpressionLine (openingParenIndex, closingParenIndex, remainderAfterCall) and added the inline step comments so intent is clearer.

Add JSDoc and high-level inline comments for call-expression analysis and stripping flow, and rename whitespace-prefixed variables for clarity.

sam-parsons · 2026-04-26T15:08:21Z

+  /**
+   * When a configured call is not a whole statement (e.g. `fn().chain()`),
+   * stripping would break runtime. Controls behavior for those lines.
+   * @default "skip"
+   */


don't need this comment in the interface, find another place for it possibly, or just delete it

Done in 385680c — removed the block comment from StripOptions and documented behavior on the ChainedCallsPolicy type instead (same text, better home for public API docs).

Move chainedCalls documentation to ChainedCallsPolicy, add step comments inside analyzeCallExpressionLine, and use clearer local variable names.

feat: handle chained-call strip edge cases safely

4c2b218

Add chainedCalls policy (skip, warn, error) and line-aware call stripping so member/call chains after a target call are not removed in ways that would break runtime.

sam-parsons commented Apr 26, 2026

View reviewed changes

chore: document chained-call analysis flow

4cbeceb

Add JSDoc and high-level inline comments for call-expression analysis and stripping flow, and rename whitespace-prefixed variables for clarity.

sam-parsons commented Apr 26, 2026

View reviewed changes

chore: clarify chainedCalls docs and call-line analysis

385680c

Move chainedCalls documentation to ChainedCallsPolicy, add step comments inside analyzeCallExpressionLine, and use clearer local variable names.

sam-parsons merged commit f390470 into main Apr 26, 2026
1 check passed

Conversation

sam-parsons commented Apr 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Issues

How to test

Notes

Learning notes

Fail-safe default for destructive transforms

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

sam-parsons commented Apr 26, 2026 •

edited

Loading