Skip to content

Conversation

@chenjiahan
Copy link
Member

Summary

Remove node:assert and replace all assertions with explicit error checks that throw descriptive errors.

Screenshot 2025-10-20 at 18 58 43

Checklist

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

@chenjiahan chenjiahan requested a review from hardfist as a code owner October 20, 2025 11:29
Copilot AI review requested due to automatic review settings October 20, 2025 11:29
@netlify
Copy link

netlify bot commented Oct 20, 2025

Deploy Preview for rspack canceled.

Name Link
🔨 Latest commit 4886469
🔍 Latest deploy log https://app.netlify.com/projects/rspack/deploys/68f632f7bcdafc0008a530d5

@github-actions github-actions bot added team The issue/pr is created by the member of Rspack. release: performance release: performance related release(mr only) labels Oct 20, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR removes node:assert usage across the codebase and replaces assertions with explicit runtime checks that throw descriptive errors.

  • Replace assert(...) with explicit if checks and throw new Error(...) in multiple modules
  • Remove assert imports and (in one case) reposition a constant declaration after imports
  • Minor string/grammar tweaks in error messages

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
packages/rspack/src/util/fs.ts Replaces an assertion in rmrf with a runtime type check; adjusts iteration over directory entries.
packages/rspack/src/rspackOptionsApply.ts Replaces assertions with explicit checks for required options; updates error messages.
packages/rspack/src/rspack.ts Adds explicit check for options.context and throws a descriptive error.
packages/rspack/src/loader-runner/index.ts Removes assert and enforces invariants in setters; moves LOADER_PROCESS_NAME after imports.
packages/rspack/src/lib/EntryOptionPlugin.ts Replaces assertion with explicit error for desc.import presence.
packages/rspack/src/config/defaults.ts Replaces assertion with explicit error for non-nil target.
packages/rspack/src/config/adapter.ts Replaces assertions with explicit errors for tsConfig type and module.defaultRules presence; validates node options.
packages/rspack/src/builtin-plugin/SplitChunksPlugin.ts Replaces assertion with explicit error when rawOptions is undefined.
packages/rspack/src/Watching.ts Replaces assertion with explicit error when compilation is missing.

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 20, 2025

📦 Binary Size-limit

Comparing 4886469 to test: use jsdom runner to run all cases (#11924) by harpsealjs

🙈 Size remains the same at 47.73MB

@codspeed-hq
Copy link

codspeed-hq bot commented Oct 20, 2025

CodSpeed Performance Report

Merging #11928 will not alter performance

Comparing remove_node_assert_1020 (4886469) with main (cf64d74)

Summary

✅ 17 untouched

@chenjiahan chenjiahan enabled auto-merge (squash) October 21, 2025 02:24
@chenjiahan chenjiahan merged commit d3cf376 into main Oct 21, 2025
94 of 100 checks passed
@chenjiahan chenjiahan deleted the remove_node_assert_1020 branch October 21, 2025 02:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release: performance release: performance related release(mr only) team The issue/pr is created by the member of Rspack.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants