Skip to content

fix: ensure consistent asset filtering with global regular expressions - #746

Open
DarshanaChandel wants to merge 2 commits into
webpack:mainfrom
DarshanaChandel:fix/global-regex-assets-filter
Open

DarshanaChandel wants to merge 2 commits into
webpack:mainfrom
DarshanaChandel:fix/global-regex-assets-filter

Conversation

@DarshanaChandel

@DarshanaChandel DarshanaChandel commented Oct 20, 2026 •

Copy link
Copy Markdown

Summary

While exploring the asset-filtering utility, I noticed a subtle inconsistency when global regular expressions (/g) are used as filter patterns.

After investigating the behavior, I found that JavaScript's RegExp.prototype.test() maintains an internal lastIndex state for global regular expressions. When the same expression is reused across multiple evaluations, this state can cause identical asset names to produce inconsistent results.

For example, repeatedly checking main.js against /\.js/g can produce alternating results, even though the input never changes. This makes the filtering behavior unpredictable and can affect which assets are included or excluded during bundle analysis.

To address this, I reset lastIndex before each regular-expression evaluation, ensuring that every check starts from a consistent state. I also added a regression test that verifies repeated evaluations return consistent results.

I kept the change focused on the underlying issue to avoid unnecessary modifications to the existing filtering logic or API.

I hope this improvement makes asset filtering more predictable and helps prevent similar issues in the future. I'd appreciate your feedback and suggestions!

What kind of change does this PR introduce?

Bug fix with a regression test.

Did you add tests for your changes?

Yes. Added a regression test in test/utils.js covering repeated asset filtering with a global regular expression.

  • npx jest test/utils.js --runInBand — 7 tests passed.
  • git diff --check — passed, with line-ending warnings only.

The complete test suite also encountered unrelated environment-dependent failures in my local Windows setup; the targeted utility tests passed.

Does this PR introduce a breaking change?

No. Existing APIs and configuration options remain unchanged.

If relevant, what needs to be documented once your changes are merged or what have you already documented?

No documentation changes are required for this internal bug fix.

Summary by CodeRabbit

  • Bug Fixes

    • Asset filtering now produces consistent results when using global regular expressions, including repeated checks of the same asset.
  • Tests

    • Added coverage to verify reliable behavior for repeated matches with global regular expressions.

@changeset-bot

changeset-bot Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: bf334da

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@linux-foundation-easycla

linux-foundation-easycla Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

CLA Signed
The committers listed above are authorized under a signed CLA.

  • ✅ login: DarshanaChandel / name: DarshanaChandel (ece1b3d)

@coderabbitai

coderabbitai Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: c4108811-ff76-4b3f-9a84-491284449e66
📥 Commits

Reviewing files that changed from the base of the PR and between ece1b3d and bf334da.

📒 Files selected for processing: 1
  • test/utils.js
🚧 Files skipped from review as they are similar to previous changes: 1
  • test/utils.js

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

Walkthrough

The asset filter resets a regular expression’s lastIndex to 0 before testing an asset. A test verifies that repeated filtering of main.js with a global regular expression returns false.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to bf334

Repeated asset filtering with a global regular expression is covered by the new test. No actionable merge risk remains after normal checks.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: consistent asset filtering with global regular expressions.
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install timed out. The project may have too many dependencies for the sandbox.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@DarshanaChandel

Copy link
Copy Markdown
Author

Greetings Sir/Ma'am!
Kindly guide me??
Maybe I did some changes as per the requirements, but if it's not fulfilled kindly help me do that, so that I can next time make better and viable changes,
Thankyou!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant