Skip to content

perf(build): use node:fs for cache checks - #7107

Open
43081j wants to merge 2 commits into
netlify:mainfrom
43081j:jg/the-purge-cache-globby
Open

perf(build): use node:fs for cache checks#7107
43081j wants to merge 2 commits into
netlify:mainfrom
43081j:jg/the-purge-cache-globby

Conversation

@43081j

@43081j 43081j commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

This switches from globby/cpy/etc to using the built-in node:fs functions.

We can use fs.cp to recursively copy, and fs.readdir to recursively read, so we no longer need dependencies to do this.


For us to review and ship your PR efficiently, please perform the following steps:

  • Open a bug/issue before writing your code 🧑‍💻. This ensures
    we can discuss the changes and get feedback from everyone that should be involved. If you`re fixing a typo or
    something that`s on fire 🔥 (e.g. incident related), you can skip this step.
  • Read the contribution guidelines 📖. This ensures
    your code follows our style guide and passes our tests.
  • Update or add tests (if any source code was changed or added) 🧪
  • Update or add documentation (if features were changed or added) 📝
  • Make sure the status checks below are successful ✅

A picture of a cute animal (not mandatory, but encouraged)

@43081j
43081j requested a review from a team as a code owner June 25, 2026 13:39
@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Improved cache file copying and detection for more reliable handling of nested directories.
    • Excluded junk files and directories during cache operations.
    • Preserved existing files when copying cached content.

Walkthrough

Updated cache filesystem utilities to use Node node:* imports, direct recursive fs.cp copying with junk filtering, and recursive fs.readdir checks for non-directory, non-junk entries. Removed the adjacent cpy and globby dependency entries from the package manifest.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: eduardoboucas

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately summarizes the main change to use built-in node:fs for cache checks.
Description check ✅ Passed The description covers the summary, motivation, and checklist items, but it omits the required Fixes #issue_number reference.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 PR with unit tests

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.

@43081j 43081j changed the title perf(build): use node:fs for cache checs perf(build): use node:fs for cache checks Jun 25, 2026

@serhalp serhalp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, just one issue

Comment thread packages/cache-utils/src/fs.ts Outdated
43081j added 2 commits July 29, 2026 15:31
This switches from globby/cpy/etc to using the built-in `node:fs` functions.

We can use `fs.cp` to recursively copy, and `fs.readdir` to recursively read, so we no longer need dependencies to do
this.
@43081j
43081j force-pushed the jg/the-purge-cache-globby branch from 4f9542e to cb1499f Compare July 29, 2026 14:37
@github-actions

Copy link
Copy Markdown
Contributor

e18e dependency analysis

No dependency warnings found.

@pkg-pr-new

pkg-pr-new Bot commented Jul 29, 2026

Copy link
Copy Markdown

Open in StackBlitz

@netlify/build

npm i https://pkg.pr.new/@netlify/build@7107

@netlify/build-info

npm i https://pkg.pr.new/@netlify/build-info@7107

@netlify/cache-utils

npm i https://pkg.pr.new/@netlify/cache-utils@7107

@netlify/config

npm i https://pkg.pr.new/@netlify/config@7107

@netlify/edge-bundler

npm i https://pkg.pr.new/@netlify/edge-bundler@7107

@netlify/functions-utils

npm i https://pkg.pr.new/@netlify/functions-utils@7107

@netlify/git-utils

npm i https://pkg.pr.new/@netlify/git-utils@7107

@netlify/headers-parser

npm i https://pkg.pr.new/@netlify/headers-parser@7107

@netlify/api

npm i https://pkg.pr.new/@netlify/api@7107

@netlify/nock-udp

npm i https://pkg.pr.new/@netlify/nock-udp@7107

@netlify/opentelemetry-sdk-setup

npm i https://pkg.pr.new/@netlify/opentelemetry-sdk-setup@7107

@netlify/opentelemetry-utils

npm i https://pkg.pr.new/@netlify/opentelemetry-utils@7107

@netlify/redirect-parser

npm i https://pkg.pr.new/@netlify/redirect-parser@7107

@netlify/run-utils

npm i https://pkg.pr.new/@netlify/run-utils@7107

@netlify/zip-it-and-ship-it

npm i https://pkg.pr.new/@netlify/zip-it-and-ship-it@7107

commit: cb1499f

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/cache-utils/src/fs.ts`:
- Around line 19-29: Update the cache copy implementation around the fs.cp call
so junk files are excluded without preventing traversal of directories or their
non-junk descendants. Remove the current filter-based behavior and pre-filter
entries or use a custom recursive walk, preserving recursive copying and the
existing destination options.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: cbada2e8-1dd3-4816-848a-e549e4862d64

📥 Commits

Reviewing files that changed from the base of the PR and between af94ccf and cb1499f.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (2)
  • packages/cache-utils/package.json
  • packages/cache-utils/src/fs.ts
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • netlify/blueprints (manual)
💤 Files with no reviewable changes (1)
  • packages/cache-utils/package.json

Comment thread packages/cache-utils/src/fs.ts
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.

2 participants