Conversation
Co-authored-by: hardfist <8898718+hardfist@users.noreply.github.com>
Co-authored-by: hardfist <8898718+hardfist@users.noreply.github.com>
Co-authored-by: hardfist <8898718+hardfist@users.noreply.github.com>
📦 Binary Size-limit
❌ Size increased by 3.38KB from 48.98MB to 48.99MB (⬆️0.01%) |
Rsdoctor Bundle Diff AnalysisFound 5 projects in monorepo, 4 projects with changes. 📊 Quick Summary
📋 Detailed Reports (Click to expand)📁 react-10kPath:
📦 Download Diff Report: react-10k Bundle Diff 📁 react-1kPath:
📦 Download Diff Report: react-1k Bundle Diff 📁 react-5kPath:
📦 Download Diff Report: react-5k Bundle Diff 📁 ui-componentsPath:
📦 Download Diff Report: ui-components Bundle Diff Generated by Rsdoctor GitHub Action |
Merging this PR will not alter performance
Comparing 
Footnotes
|
There was a problem hiding this comment.
Pull request overview
This PR fixes SwcJsMinimizerRspackPlugin’s extractComments.condition handling so that JavaScript RegExp flags are preserved and JavaScript regex features (e.g. lookaheads) work as expected (closer to terser-webpack-plugin behavior).
Changes:
- Preserve
RegExp.flagsacross the JS → binding → Rust pipeline via a newconditionFlags/condition_flagsfield. - Switch
extractComments.conditionmatching in the SWC minimizer from Rustregex::Regextorspack_regex::RspackRegex(regress-backed). - Add config-case tests covering lookaheads and case-(in)sensitive flag behavior.
Reviewed changes
Copilot reviewed 10 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/rspack-test/configCases/plugins/minify-extract-comments-regex-flags/rspack.config.js | Adds a regression test config using a lookahead + i flag regex condition. |
| tests/rspack-test/configCases/plugins/minify-extract-comments-regex-flags/index.js | Verifies extracted license output matches expected include/exclude behavior. |
| tests/rspack-test/configCases/plugins/minify-extract-comments-case-sensitive/rspack.config.js | Adds a case-sensitive regex condition test config. |
| tests/rspack-test/configCases/plugins/minify-extract-comments-case-sensitive/index.js | Asserts only uppercase matches are extracted. |
| tests/rspack-test/configCases/plugins/minify-extract-comments-case-insensitive/rspack.config.js | Adds a case-insensitive (i) regex condition test config. |
| tests/rspack-test/configCases/plugins/minify-extract-comments-case-insensitive/index.js | Asserts both uppercase/lowercase matches are extracted with i. |
| packages/rspack/src/builtin-plugin/SwcJsMinimizerPlugin.ts | Extracts and forwards regex source + flags through to raw options. |
| crates/rspack_plugin_swc_js_minimizer/src/lib.rs | Uses RspackRegex for extractComments matching and threads flags through hashing/options. |
| crates/rspack_plugin_swc_js_minimizer/Cargo.toml | Adds rspack_regex dependency for JS-compatible regex evaluation. |
| crates/rspack_binding_api/src/raw_options/raw_builtins/raw_swc_js_minimizer.rs | Extends RawExtractComments to accept condition_flags and maps it into plugin options. |
| crates/node_binding/napi-binding.d.ts | Updates binding typings to expose conditionFlags. |
| Cargo.lock | Locks the added workspace dependency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| const conditionStr = ( | ||
| condition?: ExtractCommentsCondition, | ||
| ): { source: string; flags: string } => { |
There was a problem hiding this comment.
conditionStr no longer returns a string (it now returns { source, flags }), so the name is misleading and makes the call sites harder to read. Consider renaming it to something like getConditionParts/normalizeCondition (or split into getConditionSource + getConditionFlags) to reflect what it returns.
| const mainFile = fs.readFileSync( | ||
| path.resolve(__dirname, "bundle0.js"), | ||
| "utf-8" | ||
| ); | ||
|
|
There was a problem hiding this comment.
mainFile is read from disk but never used, which adds unnecessary I/O and can confuse future readers about what the assertion depends on. Remove the read (or use it in an assertion) to keep the test focused.
| const mainFile = fs.readFileSync( | |
| path.resolve(__dirname, "bundle0.js"), | |
| "utf-8" | |
| ); | |
…s condition (#12990) * Initial plan * Add regex flags support for extractComments condition Co-authored-by: hardfist <8898718+hardfist@users.noreply.github.com> * Add tests for regex flags support in extractComments Co-authored-by: hardfist <8898718+hardfist@users.noreply.github.com> * Address code review comments Co-authored-by: hardfist <8898718+hardfist@users.noreply.github.com> * fix: cargo fmt --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: hardfist <8898718+hardfist@users.noreply.github.com> Co-authored-by: CPunisher <1343316114@qq.com>
…actComments condition (#13437) feat: support regex flags and JavaScript lookaheads in extractComments condition (#12990) * Initial plan * Add regex flags support for extractComments condition * Add tests for regex flags support in extractComments * Address code review comments * fix: cargo fmt --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: hardfist <8898718+hardfist@users.noreply.github.com>
SwcJsMinimizerRspackPlugin'sextractComments.conditionwas dropping regex flags and failing on JavaScript regex features like lookaheads, breaking patterns that worked interser-webpack-plugin.Changes
Pass regex flags through the binding layer
conditionFlagsfield toRawExtractCommentssourceandflagsfrom JavaScript RegExp objectsReplace Rust regex with JavaScript-compatible engine
rspack_regex::RspackRegex(wrapsregress) instead ofregex::RegexExample
Patterns like the following now work correctly:
Extracts
/*! important */but skips/*! SuppressStringValidation */.Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.