fix: enable requireAsExpression by default again and not to throw warning when requireAlias is disabled by LingyuCoder · Pull Request #12998 · web-infra-dev/rspack

LingyuCoder · 2026-02-09T07:50:34Z

Summary

If the requireAsExpression is disabled by default, the typeof require === "function" && require will not be convert to true && require and throw an error of require is not defined in runtime. But we can not just convert all typeof require === "function" to false because typeof require === "function" && require("./a") need it to be true. So we can only enable requireAsExpression by default.

Meanwhile, if requireAlias is disabled, the require which in var cjs = require will throw an warning. So we just need to prevent generating this warning by adding a flag to parser and if the renaming happens to require then skip the warning.

Checklist

Tests updated (or not required).
Documentation updated (or not required).

cloudflare-workers-and-pages · 2026-02-09T07:59:19Z

Deploying rspack with Cloudflare Pages

Latest commit:	`ba2c2b7`
Status:	✅ Deploy successful!
Preview URL:	https://02da3b22.rspack-v2.pages.dev
Branch Preview URL:	https://fix-umd-require-error.rspack-v2.pages.dev

View logs

Copilot

Pull request overview

This PR restores requireAsExpression to be enabled by default to avoid runtime require is not defined errors in patterns like typeof require === "function" && require, and adjusts CommonJS parsing to suppress the “critical dependency” warning specifically when require is being renamed while requireAlias is disabled.

Changes:

Enable module.parser.javascript.requireAsExpression by default and update docs to reflect the new default.
Add parser state to suppress the “unknown context critical” warning during require renaming when requireAlias is false.
Update/add test cases and snapshots to cover both requireAlias enabled/disabled behaviors under the new default.

Reviewed changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
website/docs/zh/config/module.mdx	Updates Chinese docs to show `requireAsExpression` default is `true` and how to disable it.
website/docs/en/config/module.mdx	Updates English docs to show `requireAsExpression` default is `true` and how to disable it.
packages/rspack/src/config/defaults.ts	Changes JS-side default parser option `requireAsExpression` to `true`.
crates/rspack/src/builder/mod.rs	Aligns Rust builder defaults for `require_as_expression` (and `require_alias`) with expected defaults.
crates/rspack_plugin_javascript/src/visitors/dependency/parser/mod.rs	Adds parser state to track when a rename is in progress.
crates/rspack_plugin_javascript/src/parser_plugin/common_js_imports_parse_plugin.rs	Suppresses “critical dependency” warning for `require` when it is being renamed and `requireAlias` is disabled.
tests/rspack-test/statsOutputCases/async-commons-chunk/snapshots/stats.txt	Updates stats snapshot to reflect new warning output.
tests/rspack-test/defaultsCases/default/base.js	Updates expected defaults to show `requireAsExpression: true`.
tests/rspack-test/configCases/parsing/require-alias-enabled/rspack.config.js	New fixture for `requireAlias: true`.
tests/rspack-test/configCases/parsing/require-alias-enabled/index.js	New test validating alias behavior when `requireAlias` is enabled.
tests/rspack-test/configCases/parsing/require-alias-enabled/file.js	New fixture module for require-alias-enabled case.
tests/rspack-test/configCases/parsing/require-alias-enabled/warnings.js	New expected warning patterns for require-alias-enabled case.
tests/rspack-test/configCases/parsing/require-alias-disabled/rspack.config.js	Removes explicit `requireAsExpression: false` since default is now `true`.
tests/rspack-test/configCases/parsing/require-alias-disabled/index.js	Adjusts assertions for output under new default `requireAsExpression`.
tests/rspack-test/configCases/parsing/require-alias-disabled/file.js	New fixture module for require-alias-disabled case.
tests/rspack-test/configCases/parsing/require-alias-disabled/warnings.js	New expected warning patterns for require-alias-disabled case.
crates/rspack/tests/snapshots/defaults__default_options.snap.new	Adds a new snapshot artifact file reflecting updated defaults.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tests/rspack-test/configCases/parsing/require-alias-enabled/index.js

crates/rspack_plugin_javascript/src/visitors/dependency/parser/mod.rs

github-actions · 2026-02-09T08:12:28Z

📦 Binary Size-limit

Comparing 1d6300d to refactor: wrap artifact in StealCell (#12979) by hardfist

🎉 Size decreased by 19.63KB from 48.65MB to 48.63MB (⬇️0.04%)

github-actions · 2026-02-09T08:14:47Z

Rsdoctor Bundle Diff Analysis

⚠️ Note: The latest commit (a206d2187d) does not have baseline artifacts. Using commit d1e3470729 for baseline comparison instead. If this seems incorrect, please wait a few minutes and try rerunning the workflow.

Found 5 projects in monorepo, 0 projects with changes.

📊 Quick Summary

Project	Total Size	Change
react-10k	5.7 MB	0
react-1k	825.4 KB	0
react-5k	2.7 MB	0
rome	984.2 KB	0
ui-components	2.2 MB	0

Generated by Rsdoctor GitHub Action

codspeed-hq · 2026-02-09T08:30:45Z

CodSpeed Performance Report

Merging this PR will degrade performance by 10.92%

_{Comparing fix/umd-require-error (1d6300d) with main (d1e3470)}

Summary

❌ 1 regressed benchmark
✅ 15 untouched benchmarks
⏩ 1 skipped benchmark¹

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

	Benchmark	`BASE`	`HEAD`	Efficiency
❌	`bundle@threejs-development`	734.2 ms	824.2 ms	-10.92%

1 benchmark was skipped, so the baseline result was used instead. If it was deleted from the codebase, click here and archive it to remove it from the performance reports. ↩

…ireAlias

fi3ework

reviewed by Codex locally on my device, is the warning expected here?

Problem location: crates/rspack_plugin_javascript/src/parser_plugin/common_js_imports_parse_plugin.rs:442 and crates/rspack_plugin_javascript/src/visitors/dependency/parser/walk.rs:933
Specific reason: rename() will first parser.walk_expression(expr) and then return Some(true) when requireAlias: false; however, get_var_name in _walk_iife will execute parser.walk_expression(expr) again in the rename == true branch, so the same require is walked twice.
Direct consequences:
- An extra CommonJsRequireContextDependency will be created;
- The warning suppressed by is_renaming in the first pass will "leak out" in the second pass (is_renaming has been restored), so Critical dependency warning may still occur in the IIFE scenario.
Suggested fix: In get_var_name of _walk_iife, treat rename == true as "already processed", return directly, and avoid extra walk_expression.

a minimal reproduction:

// source
(function (r) {})(require);

// config
const path = require("node:path");
/** @type {import("@rspack/core").Configuration} */
module.exports = {
  mode: "development",
  context: __dirname,
  entry: "./src/index.js",
  output: {
    path: path.resolve(__dirname, "dist"),
    filename: "bundle.js"
  },
  module: {
    parser: {
      javascript: {
        requireAlias: false,
        requireAsExpression: true,
        unknownContextCritical: true
      }
    }
  },
  infrastructureLogging: {
    level: "none"
  },
  stats: {
    preset: "errors-warnings",
    errorDetails: true
  }
};

result

WARNING in ./src/index.js 1:19-26
  ⚠ Critical dependency: require function is used in a way in which dependencies cannot be statically extracted
   ╭────
 1 │ (function (r) {})(require);
   ·                   ───────
   ╰────
Rspack compiled with 1 warning

and the fix

diff --git a/crates/rspack_plugin_javascript/src/visitors/dependency/parser/walk.rs b/crates/rspack_plugin_javascript/src/visitors/dependency/parser/walk.rs
index 30217bc8a3..96900d966a 100644
--- a/crates/rspack_plugin_javascript/src/visitors/dependency/parser/walk.rs
+++ b/crates/rspack_plugin_javascript/src/visitors/dependency/parser/walk.rs
@@ -932,15 +932,18 @@ impl JavascriptParser<'_> {
         && rename_identifier
           .call_hooks_name(parser, |this, for_name| drive.can_rename(this, for_name))
           .unwrap_or_default()
-        && !rename_identifier
+      {
+        if !rename_identifier
           .call_hooks_name(parser, |this, for_name| drive.rename(this, expr, for_name))
           .unwrap_or_default()
-      {
-        let variable = parser
-          .get_variable_info(&rename_identifier)
-          .map(|info| ExportedVariableInfo::VariableInfo(info.id()))
-          .unwrap_or(ExportedVariableInfo::Name(rename_identifier));
-        return Some(variable);
+        {
+          let variable = parser
+            .get_variable_info(&rename_identifier)
+            .map(|info| ExportedVariableInfo::VariableInfo(info.id()))
+            .unwrap_or(ExportedVariableInfo::Name(rename_identifier));
+          return Some(variable);
+        }
+        return None;
       }
       parser.walk_expression(expr);
       None

LingyuCoder · 2026-02-10T04:34:02Z

reviewed by Codex locally on my device, is the warning expected here?

Problem location: crates/rspack_plugin_javascript/src/parser_plugin/common_js_imports_parse_plugin.rs:442 and crates/rspack_plugin_javascript/src/visitors/dependency/parser/walk.rs:933
Specific reason: rename() will first parser.walk_expression(expr) and then return Some(true) when requireAlias: false; however, get_var_name in _walk_iife will execute parser.walk_expression(expr) again in the rename == true branch, so the same require is walked twice.
Direct consequences:
- An extra CommonJsRequireContextDependency will be created;
- The warning suppressed by is_renaming in the first pass will "leak out" in the second pass (is_renaming has been restored), so Critical dependency warning may still occur in the IIFE scenario.
Suggested fix: In get_var_name of _walk_iife, treat rename == true as "already processed", return directly, and avoid extra walk_expression.

a minimal reproduction:

// source
(function (r) {})(require);

// config
const path = require("node:path");
/** @type {import("@rspack/core").Configuration} */
module.exports = {
  mode: "development",
  context: __dirname,
  entry: "./src/index.js",
  output: {
    path: path.resolve(__dirname, "dist"),
    filename: "bundle.js"
  },
  module: {
    parser: {
      javascript: {
        requireAlias: false,
        requireAsExpression: true,
        unknownContextCritical: true
      }
    }
  },
  infrastructureLogging: {
    level: "none"
  },
  stats: {
    preset: "errors-warnings",
    errorDetails: true
  }
};

result

WARNING in ./src/index.js 1:19-26
  ⚠ Critical dependency: require function is used in a way in which dependencies cannot be statically extracted
   ╭────
 1 │ (function (r) {})(require);
   ·                   ───────
   ╰────
Rspack compiled with 1 warning

and the fix

diff --git a/crates/rspack_plugin_javascript/src/visitors/dependency/parser/walk.rs b/crates/rspack_plugin_javascript/src/visitors/dependency/parser/walk.rs
index 30217bc8a3..96900d966a 100644
--- a/crates/rspack_plugin_javascript/src/visitors/dependency/parser/walk.rs
+++ b/crates/rspack_plugin_javascript/src/visitors/dependency/parser/walk.rs
@@ -932,15 +932,18 @@ impl JavascriptParser<'_> {
         && rename_identifier
           .call_hooks_name(parser, |this, for_name| drive.can_rename(this, for_name))
           .unwrap_or_default()
-        && !rename_identifier
+      {
+        if !rename_identifier
           .call_hooks_name(parser, |this, for_name| drive.rename(this, expr, for_name))
           .unwrap_or_default()
-      {
-        let variable = parser
-          .get_variable_info(&rename_identifier)
-          .map(|info| ExportedVariableInfo::VariableInfo(info.id()))
-          .unwrap_or(ExportedVariableInfo::Name(rename_identifier));
-        return Some(variable);
+        {
+          let variable = parser
+            .get_variable_info(&rename_identifier)
+            .map(|info| ExportedVariableInfo::VariableInfo(info.id()))
+            .unwrap_or(ExportedVariableInfo::Name(rename_identifier));
+          return Some(variable);
+        }
+        return None;
       }
       parser.walk_expression(expr);
       None

reviewed by Codex locally on my device, is the warning expected here?

Problem location: crates/rspack_plugin_javascript/src/parser_plugin/common_js_imports_parse_plugin.rs:442 and crates/rspack_plugin_javascript/src/visitors/dependency/parser/walk.rs:933
Specific reason: rename() will first parser.walk_expression(expr) and then return Some(true) when requireAlias: false; however, get_var_name in _walk_iife will execute parser.walk_expression(expr) again in the rename == true branch, so the same require is walked twice.
Direct consequences:
- An extra CommonJsRequireContextDependency will be created;
- The warning suppressed by is_renaming in the first pass will "leak out" in the second pass (is_renaming has been restored), so Critical dependency warning may still occur in the IIFE scenario.
Suggested fix: In get_var_name of _walk_iife, treat rename == true as "already processed", return directly, and avoid extra walk_expression.

a minimal reproduction:

// source
(function (r) {})(require);

// config
const path = require("node:path");
/** @type {import("@rspack/core").Configuration} */
module.exports = {
  mode: "development",
  context: __dirname,
  entry: "./src/index.js",
  output: {
    path: path.resolve(__dirname, "dist"),
    filename: "bundle.js"
  },
  module: {
    parser: {
      javascript: {
        requireAlias: false,
        requireAsExpression: true,
        unknownContextCritical: true
      }
    }
  },
  infrastructureLogging: {
    level: "none"
  },
  stats: {
    preset: "errors-warnings",
    errorDetails: true
  }
};

result

WARNING in ./src/index.js 1:19-26
  ⚠ Critical dependency: require function is used in a way in which dependencies cannot be statically extracted
   ╭────
 1 │ (function (r) {})(require);
   ·                   ───────
   ╰────
Rspack compiled with 1 warning

and the fix

diff --git a/crates/rspack_plugin_javascript/src/visitors/dependency/parser/walk.rs b/crates/rspack_plugin_javascript/src/visitors/dependency/parser/walk.rs
index 30217bc8a3..96900d966a 100644
--- a/crates/rspack_plugin_javascript/src/visitors/dependency/parser/walk.rs
+++ b/crates/rspack_plugin_javascript/src/visitors/dependency/parser/walk.rs
@@ -932,15 +932,18 @@ impl JavascriptParser<'_> {
         && rename_identifier
           .call_hooks_name(parser, |this, for_name| drive.can_rename(this, for_name))
           .unwrap_or_default()
-        && !rename_identifier
+      {
+        if !rename_identifier
           .call_hooks_name(parser, |this, for_name| drive.rename(this, expr, for_name))
           .unwrap_or_default()
-      {
-        let variable = parser
-          .get_variable_info(&rename_identifier)
-          .map(|info| ExportedVariableInfo::VariableInfo(info.id()))
-          .unwrap_or(ExportedVariableInfo::Name(rename_identifier));
-        return Some(variable);
+        {
+          let variable = parser
+            .get_variable_info(&rename_identifier)
+            .map(|info| ExportedVariableInfo::VariableInfo(info.id()))
+            .unwrap_or(ExportedVariableInfo::Name(rename_identifier));
+          return Some(variable);
+        }
+        return None;
       }
       parser.walk_expression(expr);
       None

Seems to be a bug of parser before, i'll fix it

LingyuCoder requested review from h-a-n-a and hardfist as code owners February 9, 2026 07:50

Copilot AI review requested due to automatic review settings February 9, 2026 07:50

github-actions bot added team The issue/pr is created by the member of Rspack. release: bug fix release: bug related release(mr only) labels Feb 9, 2026

Copilot started reviewing on behalf of LingyuCoder February 9, 2026 07:51 View session

LingyuCoder force-pushed the fix/umd-require-error branch from 4a9b2b4 to de501f1 Compare February 9, 2026 07:57

Copilot AI reviewed Feb 9, 2026

View reviewed changes

tests/rspack-test/configCases/parsing/require-alias-enabled/index.js Show resolved Hide resolved

crates/rspack_plugin_javascript/src/visitors/dependency/parser/mod.rs Show resolved Hide resolved

LingyuCoder requested a review from fi3ework February 9, 2026 09:18

fix: enable requireAsExpression and not throw error when disable requ…

ba2c2b7

…ireAlias

LingyuCoder force-pushed the fix/umd-require-error branch from de501f1 to ba2c2b7 Compare February 9, 2026 09:47

clippy

646329f

LingyuCoder enabled auto-merge (squash) February 9, 2026 10:55

LingyuCoder changed the title ~~fix: enable requireAsExpression by default again and not to throw error when requireAlias is disabled~~ fix: enable requireAsExpression by default again and not to throw warning when requireAlias is disabled Feb 9, 2026

fi3ework reviewed Feb 9, 2026

View reviewed changes

fix: renaming iife require

1d6300d

fi3ework approved these changes Feb 10, 2026

View reviewed changes

LingyuCoder merged commit 4ca63ec into main Feb 10, 2026
73 of 76 checks passed

LingyuCoder deleted the fix/umd-require-error branch February 10, 2026 05:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: enable requireAsExpression by default again and not to throw warning when requireAlias is disabled#12998

fix: enable requireAsExpression by default again and not to throw warning when requireAlias is disabled#12998
LingyuCoder merged 3 commits intomainfrom
fix/umd-require-error

LingyuCoder commented Feb 9, 2026

Uh oh!

cloudflare-workers-and-pages bot commented Feb 9, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Feb 9, 2026 •

edited

Loading

Uh oh!

github-actions bot commented Feb 9, 2026 •

edited

Loading

Uh oh!

codspeed-hq bot commented Feb 9, 2026 •

edited

Loading

Uh oh!

fi3ework left a comment •

edited

Loading

Uh oh!

LingyuCoder commented Feb 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

LingyuCoder commented Feb 9, 2026

Summary

Related links

Checklist

Uh oh!

cloudflare-workers-and-pages bot commented Feb 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Deploying rspack with Cloudflare Pages

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Feb 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📦 Binary Size-limit

Uh oh!

github-actions bot commented Feb 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rsdoctor Bundle Diff Analysis

Uh oh!

codspeed-hq bot commented Feb 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CodSpeed Performance Report

Merging this PR will degrade performance by 10.92%

Summary

Performance Changes

Footnotes

Uh oh!

fi3ework left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LingyuCoder commented Feb 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cloudflare-workers-and-pages bot commented Feb 9, 2026 •

edited

Loading

github-actions bot commented Feb 9, 2026 •

edited

Loading

github-actions bot commented Feb 9, 2026 •

edited

Loading

codspeed-hq bot commented Feb 9, 2026 •

edited

Loading

fi3ework left a comment •

edited

Loading