fix(core): use enumeratorAtPath for iOS shallow entity enumeration#11182
fix(core): use enumeratorAtPath for iOS shallow entity enumeration#11182NathanWalker wants to merge 5 commits intomainfrom
Conversation
Added test case for nested file enumeration in file system tests.
|
View your CI Pipeline Execution ↗ for commit 9880fa4
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Pull request overview
Fixes a brittle iOS implementation of FileSystemAccess.enumEntities() that could crash at startup (e.g., during font registration) by switching from contentsOfDirectoryAtPathError() to enumeratorAtPath() while preserving shallow enumeration behavior.
Changes:
- Reworked iOS entity enumeration to use
NSFileManager.enumeratorAtPath()and skip descendant enumeration. - Updated
eachEntity()behavior to remain shallow by skipping directory descendants. - Added a regression test intended to ensure nested files are not enumerated.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/core/file-system/file-system-access.ios.ts | Switches iOS enumeration to enumeratorAtPath() and skips descendants to maintain shallow semantics. |
| apps/automated/src/file-system/file-system-tests.ts | Adds a regression assertion intended to ensure eachEntity() doesn’t enumerate nested files. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const enumerator = fileManager.enumeratorAtPath(path); | ||
| if (!enumerator) { | ||
| if (onError) { | ||
| onError(new Error("Failed to enum files for folder '" + path + "': " + ex)); | ||
| onError(new Error("Failed to enum files for folder '" + path + "'")); | ||
| } |
There was a problem hiding this comment.
onError now reports a generic "Failed to enum files…" message when enumeratorAtPath returns null, losing the underlying reason (e.g., non-existent path, permission error) that previously came from the thrown exception. It would be more actionable to either (1) check folderExists(path) up front and report a specific "no folder exists" error, or (2) include additional context (when available) about why enumeration failed.
There was a problem hiding this comment.
@copilot apply changes based on this feedback
There was a problem hiding this comment.
Implemented in 827cf2a. enumEntities() now checks folderExists(path) first and reports a specific missing-folder error, and the enumeratorAtPath null case now returns a more actionable message (unable to create directory enumerator).
There was a problem hiding this comment.
We likely don't want to return in this case, just print the error but allow it to proceed because this can be an innocent condition but having it in the logs will be helpful in case.
There was a problem hiding this comment.
We likely don't want to return in this case, just print the error but allow it to proceed because this can be an innocent condition but having it in the logs will be helpful in case.
@copilot adjust given this case.
There was a problem hiding this comment.
Adjusted in 9880fa4. For the benign iOS enumEntities cases, we now log diagnostics with console.error and return without invoking onError, so startup can continue while still leaving a trace in logs.
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Summary
Core's iOS
FileSystemAccess.enumEntities()implementation was brittle - this fixes a startup crash observed in some app setups during font registration.Problem
In iOS,
enumEntities()usedNSFileManager.contentsOfDirectoryAtPathError(path)and then immediately accessedfiles.count.In the failing startup path, that call could return
undefined, which caused:TypeError: Cannot read properties of undefined (reading 'count')registerFontsInFolder()Fix
contentsOfDirectoryAtPathError(path)withenumeratorAtPath(path)in the iOS file-system access layer.skipDescendants()for child directories so nested contents are not enumerated.eachEntity()does not enumerate nested files.enumeratorAtPath(path)avoids the more brittle...Errorbridge path while still giving us the directory contents we need. The implementation keepseachEntity()behavior aligned with the previous shallow directory listing semantics.