Bump rollup from 1.32.1 to 4.32.0

[amitsingh05667]

• Changed the build script to set PROD environment variable for production builds.
• Updated rollup and related plugins to their latest versions.
• Replaced deprecated rollup plugins with their new counterparts.
• Added a custom plugin to fix ES5 compatibility by replacing optional catch bindings.
• Improved commonjs plugin configuration for better module handling.

[amitsingh05667]

@claude

[amitsingh05667]

@claude

[amitsingh05667]

@claude

[github-actions]

Claude finished @amitsingh05667's task —— View job

---

Overall Assessment

❌ This PR is not ready to be merged. While the Rollup upgrade from 1.32.1 to 4.32.0 is a positive modernization effort, there are several critical issues with the custom ES5 compatibility plugin and configuration choices that need to be addressed before merging.

Key Issues Found:

Critical:

• The regex-based fixES5 plugin may corrupt string literals or comments containing "} catch {" patterns
• The ignoreDynamicRequires: true setting is dangerous and could break runtime functionality if any bundled dependencies use dynamic requires

Code Quality:

• Multiple redundant configurations that add maintenance overhead (default values being explicitly set)
• Inefficient nested regex operations in the ES5 fix plugin
• Inconsistent build flag mechanism (both env var and CLI arg)
• The ES5 plugin runs on all builds including development, adding unnecessary overhead

I've provided inline comments with specific suggestions for each issue. The most critical concern is the regex-based catch block transformation, which should ideally be replaced with a proper AST-based approach for production code targeting IE9.

---

[github-actions]

The regex pattern }\s*catch\s*\{\s*/g may match catch blocks inside string literals or comments, leading to false positives. The comment on line 32 claims to avoid this, but the regex doesn't actually prevent matching inside strings or template literals.

For production code targeting IE9, consider using a proper AST-based approach (e.g., via a Babel plugin) rather than regex-based string replacement to avoid corrupting string literals or comments that contain the pattern "} catch {".

[github-actions]

The comment claims "Safe to ignore dynamic requires since the codebase doesn't use them" and states it was "Verified via grep - no require() calls in src/". However, this is a dangerous assumption:

1. Dependencies in node_modules/ may use dynamic requires that need proper handling
2. The superagent dependency (included in Babel transforms line 84) may contain dynamic requires
3. Setting ignoreDynamicRequires: true can cause runtime errors if any bundled code uses require() with dynamic expressions

Unless you've verified that all bundled dependencies are free of dynamic requires, this setting could break the build. Consider removing this option or being more thorough in the verification.

[github-actions]

The comment justifies using defaultIsModuleExports: 'auto' for "better compatibility", but this is actually the default value in @rollup/plugin-commonjs v28.

The explicit configuration adds no value and creates maintenance overhead. Consider removing this line to rely on the plugin's default behavior, which is already 'auto'.

[github-actions]

The comment "Enable mangle for better minification in production builds" is misleading because mangle: true is already the default in terser. This explicit configuration adds no functional value.

Consider removing this line to reduce configuration noise and rely on terser's defaults.

[github-actions]

The regex replacement originalText.replace(/catch\s*\{/, 'catch (e) {') is applied to text that has already been matched by the outer regex. This creates a redundant nested regex operation.

Since originalText is guaranteed to contain the pattern from line 33's match, you can construct the replacement string directly without another regex:

Suggested change

	const replacement = originalText.replace(/catch\s*\{/, 'catch (e) {');
	const replacement = originalText.replace('catch {', 'catch (e) {');

This is more efficient and clearer.

[github-actions]

The loop modifies magicString multiple times but only returns a single source map at the end. When making multiple edits with MagicString, each overwrite call updates the internal source map state, but the source mappings may become inaccurate if the replacements overlap or affect each other's positions.

While this likely works for non-overlapping replacements, a more robust approach would be to collect all match positions first, then apply them in reverse order (end to start) to avoid position shifting issues:

const matches = [];
let match;
while ((match = catchRegex.exec(code)) !== null) {
  matches.push(match);
}

// Apply replacements in reverse order to avoid position shifting
for (let i = matches.length - 1; i >= 0; i--) {
  const match = matches[i];
  const startPos = match.index;
  const endPos = startPos + match[0].length;
  const originalText = match[0];
  const replacement = originalText.replace('catch {', 'catch (e) {');
  magicString.overwrite(startPos, endPos, replacement);
}

[github-actions]

The build script now uses cross-env PROD=true to set the production flag, but this creates an inconsistency: the rollup config also checks for argv.prod (line 59 in rollup.config.js).

This dual-checking mechanism could lead to confusion. Consider standardizing on a single approach - either environment variable OR CLI argument, not both.

[github-actions]

The fixES5 plugin only runs on UMD and ES formats, but the config also generates production builds with these exact formats (lines 116, 122). However, there's a logical issue: the plugin runs for all builds when added to getPlugins(), not just production builds.

For development builds with inline sourcemaps (line 147, 169), applying this transformation adds unnecessary overhead. Consider:

1. Only applying fixES5() to production builds where ES5 compatibility truly matters
2. Or document why development builds also need this transformation