feat(@formatjs/cli): add --follow-links flag for symlink traversal in glob patterns#6174
Merged
feat(@formatjs/cli): add --follow-links flag for symlink traversal in glob patterns#6174
Conversation
… glob patterns Fixes #6173. The Rust CLI's WalkDir does not follow symlinks by default, causing glob patterns like `node_modules/**/dist/lang/en.json` to fail with pnpm (which symlinks packages). This adds a `--follow-links` flag (default: true) to the compile, extract, and verify commands in both the Rust and TypeScript CLIs, matching fast-glob's default behavior. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Comment on lines
+1192
to
+1228
| #[test] | ||
| fn test_compile_with_node_modules_glob() { | ||
| // Regression test for #6173: glob patterns with node_modules-like structure | ||
| let dir = tempdir().unwrap(); | ||
|
|
||
| // Create node_modules/some-pkg/dist/lang/en.json | ||
| let pkg_lang = dir.path().join("node_modules/some-pkg/dist/lang"); | ||
| fs::create_dir_all(&pkg_lang).unwrap(); | ||
| fs::write( | ||
| pkg_lang.join("en.json"), | ||
| json!({"greeting": {"defaultMessage": "Hello from package!"}}).to_string(), | ||
| ) | ||
| .unwrap(); | ||
|
|
||
| let output_file = dir.path().join("compiled.json"); | ||
| let pattern = PathBuf::from(format!( | ||
| "{}/node_modules/**/dist/lang/en.json", | ||
| dir.path().display() | ||
| )); | ||
|
|
||
| compile( | ||
| &[pattern], | ||
| None, | ||
| Some(&output_file), | ||
| false, | ||
| false, | ||
| None, | ||
| false, | ||
| true, // follow links | ||
| ) | ||
| .unwrap(); | ||
|
|
||
| let output_content = fs::read_to_string(&output_file).unwrap(); | ||
| let output_json: serde_json::Value = serde_json::from_str(&output_content).unwrap(); | ||
|
|
||
| assert_eq!(output_json["greeting"], "Hello from package!"); | ||
| } |
There was a problem hiding this comment.
The test test_compile_with_node_modules_glob does not actually test symlink traversal despite being described as a "regression test for #6173" which is about symlinked packages. The test creates a regular directory structure with fs::create_dir_all() rather than actual symlinks. This means the test will pass regardless of the follow_links parameter value and does not verify the intended behavior.
To properly test symlink following:
// Create actual symlinks
#[cfg(unix)]
use std::os::unix::fs::symlink;
#[cfg(windows)]
use std::os::windows::fs::symlink_dir as symlink;
let target_dir = dir.path().join("actual-pkg/dist/lang");
fs::create_dir_all(&target_dir).unwrap();
fs::write(
target_dir.join("en.json"),
json!({"greeting": {"defaultMessage": "Hello from package!"}}).to_string(),
).unwrap();
// Create symlink in node_modules
let node_modules = dir.path().join("node_modules");
fs::create_dir_all(&node_modules).unwrap();
symlink(dir.path().join("actual-pkg"), node_modules.join("some-pkg")).unwrap();
// Test with follow_links=true succeeds
// Test with follow_links=false finds no files
Suggested change
| #[test] | |
| fn test_compile_with_node_modules_glob() { | |
| // Regression test for #6173: glob patterns with node_modules-like structure | |
| let dir = tempdir().unwrap(); | |
| // Create node_modules/some-pkg/dist/lang/en.json | |
| let pkg_lang = dir.path().join("node_modules/some-pkg/dist/lang"); | |
| fs::create_dir_all(&pkg_lang).unwrap(); | |
| fs::write( | |
| pkg_lang.join("en.json"), | |
| json!({"greeting": {"defaultMessage": "Hello from package!"}}).to_string(), | |
| ) | |
| .unwrap(); | |
| let output_file = dir.path().join("compiled.json"); | |
| let pattern = PathBuf::from(format!( | |
| "{}/node_modules/**/dist/lang/en.json", | |
| dir.path().display() | |
| )); | |
| compile( | |
| &[pattern], | |
| None, | |
| Some(&output_file), | |
| false, | |
| false, | |
| None, | |
| false, | |
| true, // follow links | |
| ) | |
| .unwrap(); | |
| let output_content = fs::read_to_string(&output_file).unwrap(); | |
| let output_json: serde_json::Value = serde_json::from_str(&output_content).unwrap(); | |
| assert_eq!(output_json["greeting"], "Hello from package!"); | |
| } | |
| #[test] | |
| fn test_compile_with_node_modules_glob() { | |
| // Regression test for #6173: glob patterns with node_modules-like structure | |
| #[cfg(unix)] | |
| use std::os::unix::fs::symlink; | |
| #[cfg(windows)] | |
| use std::os::windows::fs::symlink_dir as symlink; | |
| let dir = tempdir().unwrap(); | |
| // Create actual package directory structure | |
| let target_dir = dir.path().join("actual-pkg/dist/lang"); | |
| fs::create_dir_all(&target_dir).unwrap(); | |
| fs::write( | |
| target_dir.join("en.json"), | |
| json!({"greeting": {"defaultMessage": "Hello from package!"}}).to_string(), | |
| ) | |
| .unwrap(); | |
| // Create symlink in node_modules | |
| let node_modules = dir.path().join("node_modules"); | |
| fs::create_dir_all(&node_modules).unwrap(); | |
| symlink(dir.path().join("actual-pkg"), node_modules.join("some-pkg")).unwrap(); | |
| let output_file = dir.path().join("compiled.json"); | |
| let pattern = PathBuf::from(format!( | |
| "{}/node_modules/**/dist/lang/en.json", | |
| dir.path().display() | |
| )); | |
| // Test with follow_links=true should succeed | |
| compile( | |
| &[pattern.clone()], | |
| None, | |
| Some(&output_file), | |
| false, | |
| false, | |
| None, | |
| false, | |
| true, // follow links | |
| ) | |
| .unwrap(); | |
| let output_content = fs::read_to_string(&output_file).unwrap(); | |
| let output_json: serde_json::Value = serde_json::from_str(&output_content).unwrap(); | |
| assert_eq!(output_json["greeting"], "Hello from package!"); | |
| // Test with follow_links=false should find no files | |
| let output_file_no_follow = dir.path().join("compiled_no_follow.json"); | |
| compile( | |
| &[pattern], | |
| None, | |
| Some(&output_file_no_follow), | |
| false, | |
| false, | |
| None, | |
| false, | |
| false, // don't follow links | |
| ) | |
| .unwrap(); | |
| let output_content_no_follow = fs::read_to_string(&output_file_no_follow).unwrap(); | |
| let output_json_no_follow: serde_json::Value = serde_json::from_str(&output_content_no_follow).unwrap(); | |
| assert_eq!(output_json_no_follow, serde_json::Value::Object(serde_json::Map::new())); | |
| } | |
Spotted by Graphite
Is this helpful? React 👍 or 👎 to let us know.
Creates the node_modules directory structure on the fly in a temp dir to avoid Bazel glob() excluding node_modules directories. This reproduces the exact scenario from the issue. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…n test Creates a temp dir mimicking pnpm's layout where the real package lives in node_modules/.pnpm/some-pkg@1.0.0/node_modules/some-pkg and node_modules/some-pkg is a symlink to it. This is the exact scenario from #6173 that fails without --follow-links. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The static fixture wasn't available in the remote Bazel executor. The on-the-fly node_modules and pnpm symlink tests cover the same scenario without relying on static fixtures. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #6173.
--follow-linksflag (default:true) tocompile,extract, andverifycommands in both Rust and TypeScript CLIsWalkDirdoes not follow symlinks by default, causing glob patterns likenode_modules/**/dist/lang/en.jsonto fail with pnpm (which symlinks packages innode_modules)trueto matchfast-glob's default behavior; users can pass--no-follow-linksto disable (useful in hermetic build systems like Bazel where symlinks may escape the sandbox)Changes
Rust CLI (
crates/formatjs_cli/src/):main.rs— added--follow-linksarg (default true) to Compile, Extract, Verify commandscompile.rs— threadedfollow_linkstoWalkDir::new().follow_links(); addedtest_compile_with_node_modules_globunit testextract.rs— threadedfollow_linkstoresolve_files_from_globs()→WalkDirverify.rs— threadedfollow_linkstoWalkDircompile_folder.rs— passedtruefor the new paramTypeScript CLI (
packages/cli-lib/src/):cli.ts— added--follow-linksoption to extract, compile, verify commands; passesfollowSymbolicLinkstoglobSynccompile.ts,extract.ts,verify/index.ts— addedfollowLinksto type definitionsIntegration tests:
compile glob with nested directory structuretest that runs on both TS and Rust CLIs--follow-linksflagTest plan
bazel test //...)test_compile_with_node_modules_glob)cli-lib🤖 Generated with Claude Code