Evaluate glob utility

[DavidWells]

Evaluate https://github.com/DavidWells/packages/tree/master/packages/util-globs/src code and its tests and see if you see any potential issues

Also add any missing test cases you think we might need

thanks @claude

[github-actions]

📋 Claude Code execution complete ✅

🎉 Automatic PR created successfully!

• PR: #19
• Branch: claude/issue-18-20251127-0514
• Next step: Review PR → Merge

📊 Execution Info: Issue #18 | React (310 files) | @DavidWells
ℹ️ Action Run: https://github.com/DavidWells/packages/actions/runs/19725954286
📝 PR Changes: https://github.com/DavidWells/packages/pull/19/files

[github-actions]

Claude finished @DavidWells's task —— View job • claude/issue-18-20251127-0514 • Create PR ➔

---

Code Review & Test Enhancement

Tasks

• Check for repository-specific instructions (CLAUDE.md)
• Explore util-globs package structure
• Review source code for potential issues
• Review existing tests
• Identify missing test cases
• Implement missing test cases
• Run tests to verify changes
• Commit and push changes

---

Code Review Findings

I've completed a comprehensive review of the glob utility code in packages/util-globs/src. Here are the key findings:

Critical Issues

1. Missing import - src/index.js:19-35: The findUp function calls escalade() but the function is defined locally below (lines 276-290). This is actually fine as the function is defined in the same file, but it would be clearer to have it defined before use or use proper hoisting comments.

2. Incomplete regex for hidden files - src/index.js:13: The regex /(^|[\\\/])\.[^\\\/\.]/g has the global flag g which can cause issues with .test() method due to stateful regex matching. The lastIndex property will affect subsequent calls.

3. Potential race condition - src/utils/get-gitignore.js:3: The function always reads from a fixed path and doesn't search upward in the directory tree for .gitignore files in parent directories.

Moderate Issues

4. Inefficient array filtering - src/index.js:262-264: Uses indexOf() instead of a Set for better O(1) lookup performance with large arrays.

5. Missing error handling - src/index.js:278-289: The escalade function doesn't handle errors from fs.stat() or fs.readdir().

6. Deprecated string method - src/index.js:341,342,344,360,364: Uses .substr() which is deprecated. Should use .substring() or .slice().

7. Inconsistent case-insensitive handling - src/index.js:79,373: The caseInsensitive flag creates flags for new regex, but some hardcoded regexes may not respect this option.

Minor Issues

8. Path normalization inconsistency - src/utils/get-gitignore.js:9: Removes leading/trailing slashes but doesn't consistently normalize ./ prefixes.

9. Code cleanliness - Multiple blocks of commented-out code should be removed for clarity.

10. No path traversal protection - The code doesn't validate that resolved paths stay within expected boundaries.

Test Coverage Added

I've added 9 new test cases in tests/additional.test.js covering:

• ✅ Error handling for missing .gitignore files
• ✅ Empty file handling in getGitignoreContents
• ✅ Comment filtering in gitignore parsing
• ✅ String vs array pattern handling consistency
• ✅ Empty patterns array behavior
• ✅ Non-existent directory handling
• ✅ Absolute vs relative path conversion
• ✅ Negated pattern support
• ✅ Concurrent API calls

All 29 tests now pass successfully!

Summary

The glob utility code is functional but has some areas for improvement:

• Most critical: Fix the global regex flag issue with IS_HIDDEN_FILE
• Recommended: Replace deprecated .substr() calls
• Nice to have: Add error handling in escalade() and improve performance with Sets

The new test cases significantly improve coverage for edge cases and error conditions that weren't previously tested.