Skip to content

fix: exclude hidden dir from snapshot managedPaths default value #12486

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.

Sign up for GitHub

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

Jump to bottom
Merged
jerrykingxyz merged 3 commits into from
Dec 17, 2025
Merged

fix: exclude hidden dir from snapshot managedPaths default value #12486

jerrykingxyz merged 3 commits into from
Dec 17, 2025

Conversation

@jerrykingxyz
Copy link
Contributor

@jerrykingxyz jerrykingxyz commented Dec 17, 2025 •
edited
Loading

Summary

Update the default managedPaths regex from //node_modules// to //node_modules/[^.]/ to exclude directories starting with a dot (e.g., .cache, .test) from being treated as managed paths in snapshot handling.

Related links

Checklist

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

Copilot AI review requested due to automatic review settings December 17, 2025 06:54
@github-actions github-actions bot added release: bug fix release: bug related release(mr only) team The issue/pr is created by the member of Rspack. labels Dec 17, 2025
@netlify
Copy link

netlify bot commented Dec 17, 2025 •
edited
Loading

✅ Deploy Preview for rspack ready!

Name Link
🔨 Latest commit c97680d
🔍 Latest deploy log https://app.netlify.com/projects/rspack/deploys/694264d8d8255d0008f18959
😎 Deploy Preview https://deploy-preview-12486--rspack.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

Copilot AI reviewed Dec 17, 2025
View reviewed changes
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 fixes the default value of snapshot.managedPaths to exclude hidden directories (starting with .) within node_modules from being treated as managed paths. This ensures that hidden directories like .test, .bin, and .cache are not cached across builds.

  • Changed the default managedPaths regex from /\/node_modules\// to /\/node_modules\/[^.]/
  • Added comprehensive test case to verify the fix works correctly

Reviewed changes

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

Show a summary per file
File Description
packages/rspack/src/config/normalization.ts Updated default managedPaths regex to exclude hidden directories by requiring a non-dot character after /node_modules/
tests/rspack-test/cacheCases/snapshot/default_value/rspack.config.js Added test configuration enabling persistent cache
tests/rspack-test/cacheCases/snapshot/default_value/index.js Added test to verify managed paths (test_lib) are cached while hidden directories (.test) are not
tests/rspack-test/cacheCases/snapshot/default_value/node_modules/test_lib/* Test fixture for regular package that should be managed
tests/rspack-test/cacheCases/snapshot/default_value/node_modules/.test/index.js Test fixture for hidden directory that should not be managed

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

@github-actions
Copy link
Contributor

github-actions bot commented Dec 17, 2025 •
edited
Loading

Rsdoctor Bundle Diff Analysis

Found 5 project(s) in monorepo.

📊 Quick Summary (Click to expand)
Project Total Size Change
react-10k 5.7 MB ➡️ 0 B (0.0%)
react-1k 823.4 KB ➡️ 0 B (0.0%)
react-5k 2.7 MB ➡️ 0 B (0.0%)
ui-components 2.1 MB ➡️ 0 B (0.0%)
rome 984.3 KB ➡️ 0 B (0.0%)
📋 Detailed Reports (Click to expand)

📁 react-10k

Path: ../build-tools-performance/cases/react-10k/dist/rsdoctor-data.json

📌 Baseline Commit: aaf1c4907f | PR: #12483

Metric Current Baseline Change
📊 Total Size 5.7 MB 5.7 MB 0 B (0.0%)
📄 JavaScript 5.7 MB 5.7 MB 0 B (0.0%)
🎨 CSS 21.0 B 21.0 B 0 B (0.0%)
🌐 HTML 0 B 0 B N/A
📁 Other Assets 0 B 0 B N/A

📦 Download Diff Report: react-10k Bundle Diff

📁 react-1k

Path: ../build-tools-performance/cases/react-1k/dist/rsdoctor-data.json

📌 Baseline Commit: aaf1c4907f | PR: #12483

Metric Current Baseline Change
📊 Total Size 823.4 KB 823.4 KB 0 B (0.0%)
📄 JavaScript 823.4 KB 823.4 KB 0 B (0.0%)
🎨 CSS 0 B 0 B N/A
🌐 HTML 0 B 0 B N/A
📁 Other Assets 0 B 0 B N/A

📦 Download Diff Report: react-1k Bundle Diff

📁 react-5k

Path: ../build-tools-performance/cases/react-5k/dist/rsdoctor-data.json

📌 Baseline Commit: aaf1c4907f | PR: #12483

Metric Current Baseline Change
📊 Total Size 2.7 MB 2.7 MB 0 B (0.0%)
📄 JavaScript 2.7 MB 2.7 MB 0 B (0.0%)
🎨 CSS 21.0 B 21.0 B 0 B (0.0%)
🌐 HTML 0 B 0 B N/A
📁 Other Assets 0 B 0 B N/A

📦 Download Diff Report: react-5k Bundle Diff

📁 ui-components

Path: ../build-tools-performance/cases/ui-components/dist/rsdoctor-data.json

📌 Baseline Commit: aaf1c4907f | PR: #12483

Metric Current Baseline Change
📊 Total Size 2.1 MB 2.1 MB 0 B (0.0%)
📄 JavaScript 2.0 MB 2.0 MB 0 B (0.0%)
🎨 CSS 83.0 KB 83.0 KB 0 B (0.0%)
🌐 HTML 0 B 0 B N/A
📁 Other Assets 0 B 0 B N/A

📦 Download Diff Report: ui-components Bundle Diff

📁 rome

Path: ../build-tools-performance/cases/rome/dist/rsdoctor-data.json

📌 Baseline Commit: aaf1c4907f | PR: #12483

Metric Current Baseline Change
📊 Total Size 984.3 KB 984.3 KB 0 B (0.0%)
📄 JavaScript 984.3 KB 984.3 KB 0 B (0.0%)
🎨 CSS 0 B 0 B N/A
🌐 HTML 0 B 0 B N/A
📁 Other Assets 0 B 0 B N/A

📦 Download Diff Report: rome Bundle Diff

Generated by Rsdoctor GitHub Action

@github-actions
Copy link
Contributor

github-actions bot commented Dec 17, 2025 •
edited
Loading

📦 Binary Size-limit

Comparing c97680d to feat: stabilize SubresourceIntegrityPlugin (#12483) by harpsealjs

🙈 Size remains the same at 48.29MB

@stormslowly
Copy link
Contributor

stormslowly commented Dec 17, 2025

Should we consider node_modules/.pnpm ?

@codspeed-hq
Copy link

codspeed-hq bot commented Dec 17, 2025 •
edited
Loading

CodSpeed Performance Report

Merging #12486 will not alter performance

Comparing jerry/pc (c97680d) with main (aaf1c49)

Summary

✅ 17 untouched

chenjiahan
chenjiahan reviewed Dec 17, 2025
View reviewed changes
@jerrykingxyz jerrykingxyz enabled auto-merge (squash) December 17, 2025 08:38
@jerrykingxyz
Copy link
Contributor Author

jerrykingxyz commented Dec 17, 2025 •
edited
Loading

Should we consider node_modules/.pnpm ?

For pnpm, the directory is usually
<project_root>/node_modules/.pnpm/<lib>/node_modules/<lib>/package.json
The node_modules in the second layer can be matched

For yarn pnp,
<project_root>/.yarn/unplugged/<lib>/node_modules/<lib>/package.json
also contains a node_modules that can be matched, so there is basically no problem

@jerrykingxyz jerrykingxyz merged commit 8b16f44 into main Dec 17, 2025
52 checks passed
@jerrykingxyz jerrykingxyz deleted the jerry/pc branch December 17, 2025 08:44
@github-actions github-actions bot mentioned this pull request Dec 17, 2025
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@stormslowly stormslowly stormslowly approved these changes

@hardfist hardfist Awaiting requested review from hardfist hardfist is a code owner

@chenjiahan chenjiahan Awaiting requested review from chenjiahan

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jerrykingxyz @stormslowly @chenjiahan