feat: add upward config discovery and enhanced error messages #1235
Conversation
Improve UX when running CLI commands from subdirectories by implementing automatic upward config search (similar to Git) and providing helpful error messages when config files are found in subdirectories. Key improvements: - Commands now search parent directories for i18n.json (like Git) - Users can run commands from any subdirectory within their project - Error messages list configs found in subdirectories with guidance - File operations resolve paths relative to config root - Consistent error handling across all commands 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
davidturnbull
force-pushed
the
feat/upward-config-discovery
branch
from
f136cea to
96ac8a5
Compare
|
@copilot does it make sense to use cosmiconfig here? WDYT? |
|
@maxprilutskiy I've opened a new pull request, #1756, to work on those changes. Once the pull request is ready, I'll request review from you. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| entry.name === ".git" || | ||
| entry.name === "dist" || | ||
| entry.name === "build" || | ||
| entry.name.startsWith(".") |
There was a problem hiding this comment.
The function silently skips directories starting with a dot (line 100), but this excludes legitimate use cases like .github, .vscode, or other dot-prefixed project directories that might contain i18n configs. While this is reasonable for most cases, consider documenting this behavior in the function's JSDoc or making it configurable.
| let _cachedConfigPath: string | null = null; | ||
| let _cachedConfigRoot: string | null = null; |
There was a problem hiding this comment.
The module-level cache variables can become stale when process.chdir() is called (which happens in CI flows at packages/cli/src/cli/cmd/ci/flows/in-branch.ts line 131 and packages/cli/src/cli/cmd/ci/platforms/gitlab.ts line 14). After a directory change, the cached path would still point to the old location relative to the new cwd, causing config resolution to fail or use the wrong config file. Consider clearing the cache when the working directory changes, or store absolute paths instead of relying on process.cwd().
| throw new Error( | ||
| `i18n.json not found in current directory or parent directories.\n\n` + | ||
| `Found ${foundBelow.length} config file(s) in subdirectories:\n` + | ||
| configList + | ||
| moreText + | ||
| `\n\nPlease cd into one of these directories, or run \`lingo.dev init\` to initialize a new project.`, | ||
| ); | ||
| } else { | ||
| throw new Error( | ||
| `i18n.json not found. Please run \`lingo.dev init\` to initialize the project.`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
The error message format is inconsistent with the standard error structure used elsewhere in the codebase. Other commands use CLIError or ConfigError with message and docUrl properties (see status.ts, i18n.ts). This plain Error without a docUrl prevents users from accessing documentation links that could help resolve the issue.
| export function saveConfig(config: I18nConfig) { | ||
| const configFilePath = _getConfigFilePath(); | ||
| const configInfo = _findConfigPath(); | ||
| if (!configInfo) { | ||
| throw new Error("Cannot save config: i18n.json not found"); | ||
| } | ||
|
|
||
| const serialized = JSON.stringify(config, null, 2); | ||
| fs.writeFileSync(configFilePath, serialized); | ||
| fs.writeFileSync(configInfo.configPath, serialized); | ||
|
|
||
| return config; | ||
| } |
There was a problem hiding this comment.
The saveConfig function will fail silently in a race condition scenario: if the config file is deleted between the _findConfigPath() check and the fs.writeFileSync() call, the write will create the file in a potentially incorrect location (wherever the last found config was). Consider using the configPath from the initial config load or validating the path still exists before writing.
| export function findConfigsDownwards( | ||
| startDir: string = process.cwd(), | ||
| maxDepth: number = 3, | ||
| ): string[] { | ||
| const found: string[] = []; | ||
|
|
||
| function search(dir: string, depth: number) { | ||
| if (depth > maxDepth) return; | ||
|
|
||
| try { | ||
| const entries = fs.readdirSync(dir, { withFileTypes: true }); | ||
|
|
||
| for (const entry of entries) { | ||
| if (entry.isDirectory()) { | ||
| // Skip common directories that shouldn't contain configs | ||
| if ( | ||
| entry.name === "node_modules" || | ||
| entry.name === ".git" || | ||
| entry.name === "dist" || | ||
| entry.name === "build" || | ||
| entry.name.startsWith(".") | ||
| ) { | ||
| continue; | ||
| } | ||
|
|
||
| const subDir = path.join(dir, entry.name); | ||
| const configPath = path.join(subDir, "i18n.json"); | ||
|
|
||
| if (fs.existsSync(configPath)) { | ||
| found.push(path.relative(startDir, configPath)); | ||
| } | ||
|
|
||
| search(subDir, depth + 1); | ||
| } | ||
| } | ||
| } catch (error) { | ||
| // Ignore permission errors, etc. | ||
| } | ||
| } | ||
|
|
||
| search(startDir, 0); | ||
| return found; | ||
| } |
There was a problem hiding this comment.
The findConfigsDownwards function has quadratic time complexity due to checking fs.existsSync() for every subdirectory individually. In a monorepo with many directories, this could cause significant performance degradation. Consider collecting all directories first, then batch-checking for config files, or using a more efficient traversal strategy.
| } | ||
|
|
||
| const fileContents = fs.readFileSync(configFilePath, "utf8"); | ||
| const { configPath, configRoot } = configInfo; |
There was a problem hiding this comment.
Unused variable configRoot.
| const { configPath, configRoot } = configInfo; | |
| const { configPath } = configInfo; |
Problem
Users must run CLI commands from the exact directory containing
i18n.json, which creates a poor user experience:lingo.dev runfrom a subdirectory fails with a generic errorsrc/,components/, etc.) is brokenBefore:
Solution
Commands now automatically search parent directories for
i18n.json(similar to how Git finds.git) and provide helpful guidance when the config isn't found.After:
Key Improvements
i18n.jsonor reach the filesystem rootrun,status,i18n) behave identicallyTesting Instructions
Test 1: Run from subdirectory
Test 2: Helpful error messages
Test 3: Monorepo scenario
Test 4: All commands work consistently
Behavior Notes
What's Changed
utils/config.tsgetConfigOrThrow()for consistent error handling