-
Notifications
You must be signed in to change notification settings - Fork 738
Fix #3483 Updater now streams files one-by-one, skipping non-source dirs #3630
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
base: main
Are you sure you want to change the base?
Conversation
…en/large dirs (node_modules, vendor, docs, etc.) to avoid OOMs and hangs; frees ASTs immediately. - ts-morph dependency bumped to ^27.0.2.
packages/update/src/update.ts
Outdated
| let totalFiles = 0; | ||
| let userFileCount = 0; | ||
| for await ( | ||
| const entry of walk(dir, { | ||
| includeDirs: false, | ||
| includeFiles: true, | ||
| exts: ["js", "jsx", "ts", "tsx"], | ||
| skip: [SKIP_DIRS], | ||
| }) | ||
| ) { | ||
| totalFiles++; | ||
| if (!HIDE_FILES.test(entry.path)) userFileCount++; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's store the found paths, so that we don't need to walk the file system again a second time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. I just pushed a new commit.
…again a second time
| const HIDE_FILES = /[\\/]+(node_modules|vendor)[\\/]+/; | ||
| // Directories we should completely skip when walking the tree. This keeps us | ||
| // from loading vendored dependencies (often thousands of files) into ts-morph. | ||
| const SKIP_DIRS = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the difference between HIDE_FILES and SKIP_DIRS? Aren't both used for the same thing? Let's unify that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup, done.
Fix system crash (OOM) caused by updater spending time in folders like docs/