Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: watchOptions.ignored use array glob instead of regexp #4075

Closed
wants to merge 2 commits into from

Conversation

2heal1
Copy link
Member

@2heal1 2heal1 commented Nov 27, 2024

Summary

Both Webpack and Rspack have the watchOptions.ignore type as Regexp | string | string[] , if the internal configuration is Regexp , it's difficult for users to expand this config, so recommend to use array instead.

before

const rsbuildConfig = watchOptions.ignored;
watchOptions.ignored = [ transform(rsbuildConfig), userConfig ]

after

const rsbuildConfig = watchOptions.ignored;
watchOptions.ignored = [ ...rsbuildConfig, userConfig ]

Checklist

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

Copy link

netlify bot commented Nov 27, 2024

Deploy Preview for rsbuild ready!

Name Link
🔨 Latest commit 9dc0463
🔍 Latest deploy log https://app.netlify.com/sites/rsbuild/deploys/6746cad7db0c5a00086dd63f
😎 Deploy Preview https://deploy-preview-4075--rsbuild.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 73 (no change from production)
Accessibility: 97 (no change from production)
Best Practices: 100 (no change from production)
SEO: 100 (no change from production)
PWA: 60 (no change from production)
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Member

@chenjiahan chenjiahan left a comment

Choose a reason for hiding this comment

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

I actually used RegExp instead of array intentionally for several reasons:

  1. I prefer to allow users to "override" the default value rather than "merge with" the default value. Since some users may want to watch node_modules, it should be easy for them to override the default behavior.
  2. RegExp has better performance. When using glob patterns, Rspack will use glob-to-regexp to generate a complex RegExp, which may slow down the build.

@2heal1
Copy link
Member Author

2heal1 commented Nov 28, 2024

gotcha , in fact i just need to modify webpack config , and i find the issue seems to be solved . Okay i will close this pr

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

Successfully merging this pull request may close these issues.

2 participants