-
Notifications
You must be signed in to change notification settings - Fork 15
feat(config): Support cedar.toml #1000
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
❌ Deploy Preview for cedarjs failed.
|
Greptile SummaryThis PR adds support for Key changes:
The core Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant CLI as CLI/Command
participant Helper as cli-helpers
participant ConfigPath as configPath.ts
participant FindUp as findUp.ts
participant FS as FileSystem
CLI->>Helper: setRedwoodCWD(cwd?)
alt cwd specified
Helper->>FS: Check for cedar.toml or redwood.toml
FS-->>Helper: exists/not exists
else cwd not specified
Helper->>FindUp: findUp('cedar.toml')
FindUp->>FS: Walk up dirs looking for cedar.toml
FS-->>FindUp: result or null
alt cedar.toml not found
Helper->>FindUp: findUp('redwood.toml')
FindUp->>FS: Walk up dirs again
FS-->>FindUp: result or null
end
end
Note over CLI,FS: Core config resolution (optimized)
CLI->>ConfigPath: getConfigPath(cwd)
ConfigPath->>FindUp: findUp(['cedar.toml', 'redwood.toml'], cwd)
loop Each directory level
FindUp->>FS: exists(cedar.toml)?
alt cedar.toml exists
FS-->>FindUp: path to cedar.toml
FindUp-->>ConfigPath: return path
else cedar.toml missing
FindUp->>FS: exists(redwood.toml)?
alt redwood.toml exists
FS-->>FindUp: path to redwood.toml
FindUp-->>ConfigPath: return path
else both missing
Note over FindUp: Move up one directory
end
end
end
ConfigPath-->>CLI: config file path
|
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.
62 files reviewed, 2 comments
| const configTomlPath = | ||
| findUp('cedar.toml', process.cwd()) || | ||
| findUp('redwood.toml', process.cwd()) |
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.
style: inefficient sequential filesystem traversals - findUp walks the directory tree twice when only redwood.toml exists
| const configTomlPath = | |
| findUp('cedar.toml', process.cwd()) || | |
| findUp('redwood.toml', process.cwd()) | |
| const configTomlPath = findUp(['cedar.toml', 'redwood.toml'], process.cwd()) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/cli-helpers/src/lib/project.ts
Line: 192:194
Comment:
**style:** inefficient sequential filesystem traversals - `findUp` walks the directory tree twice when only `redwood.toml` exists
```suggestion
const configTomlPath = findUp(['cedar.toml', 'redwood.toml'], process.cwd())
```
How can I resolve this? If you propose a fix, please make it concise.
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run-many -t test:types |
✅ Succeeded | 6s | View ↗ |
nx run-many -t build:pack --exclude create-ceda... |
✅ Succeeded | 33s | View ↗ |
nx run-many -t build |
✅ Succeeded | 2m 3s | View ↗ |
☁️ Nx Cloud last updated this comment at 2026-01-15 14:20:01 UTC
|
@greptileai Can you please review this PR again and pay extra attention to any changes that might cause problems on Windows systems? |
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.
Looking at this PR, I can identify several potential issues that could cause problems on Windows systems:
1. Path Separator Issues in Docker Templates
In packages/cli/src/commands/setup/docker/templates/Dockerfile, the COPY commands use globbing patterns that may not work consistently across platforms:
COPY --chown=node:node cedar.toml* redwood.toml* ./This pattern assumes Unix-style path handling and may not work reliably on Windows Docker contexts.
2. File Path Construction in Configuration Detection
In packages/cli/src/lib/project.ts and packages/project-config/src/configPath.ts, there are multiple places where file paths are constructed and checked:
const configFiles = ['cedar.toml', 'redwood.toml']
if (!configFiles.some((file) => cwd && fs.existsSync(path.join(cwd, file)))) {While path.join() should handle this correctly, the error messages and path comparisons might not account for Windows path separators.
3. Path Comparison and Caching Issues
In packages/project-config/src/configPath.ts, the caching mechanism uses the cwd as a key:
const getConfigPathCache = new Map<string, string>()On Windows, path casing and separator differences could cause cache misses or incorrect lookups.
4. Find-up Logic Potential Issues
The find-up logic now checks for two files instead of one:
const configPath = findUp('cedar.toml', cwd) || findUp('redwood.toml', cwd)This could be inefficient on Windows and may have different behavior depending on the file system case sensitivity.
5. Template Path References
Several template files now reference both config files with wildcards, which might not expand correctly on Windows systems, particularly in the Docker context.
Recommendations
To make this more Windows-compatible, consider:
- Normalize paths consistently - ensure all path operations use
path.resolve()andpath.normalize() - Test Docker globbing - verify the
cedar.toml* redwood.toml*pattern works on Windows Docker Desktop - Case-insensitive caching - normalize cache keys for path comparisons on Windows
- Error message paths - ensure error messages display paths in the correct format for the platform
The changes look generally solid, but these Windows-specific considerations should be tested thoroughly, especially the Docker template changes and the dual-file detection logic.
|
@greptileai We don't need Docker support on Windows, so ignore that. I'm specifically seeing this error. Can you spot what change might have caused this? |
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.
Looking at the error, the issue appears to be a TypeScript compilation error in the @cedarjs/internal package, specifically in src/routes.ts. The error shows:
Could not find a declaration file for module '@cedarjs/structure'Unused '@ts-expect-error' directive
However, looking through the diff you provided, I don't see any changes to the @cedarjs/structure package or the @cedarjs/internal package that would directly cause this issue.
The most likely culprit is this change in File 67: packages/structure/src/model/RWProject.ts:
@lazy() get redwoodTOML(): RWTOML {
- return new RWTOML(join(this.projectRoot, 'redwood.toml'), this)
+ return new RWTOML(getConfigPath(this.projectRoot), this)
}And the related import addition:
import {
getPaths,
processPagesDir,
getSchemaPath,
+ getConfigPath,
} from '@cedarjs/project-config'This change modifies the public API of the @cedarjs/structure package by changing how the RWTOML class is instantiated. The getConfigPath() function now looks for either cedar.toml or redwood.toml, which could potentially return a different path or throw an error in cases where it previously worked.
The TypeScript error about missing declaration files suggests that there might be a build order issue or the @cedarjs/structure package needs to be rebuilt before @cedarjs/internal can properly import it.
The "Unused '@ts-expect-error' directive" error in src/routes.ts line 102 suggests that whatever error was being suppressed there is no longer occurring, possibly due to the API change in the structure package.
Recommendation: Try rebuilding the @cedarjs/structure package first, then rebuild @cedarjs/internal. The build system might need the structure package to be built with its new API before other packages can properly reference it.

External vendors, and AI tools, use
redwood.tomlto detect RW apps. As Cedar adds more and more features not available in RW it's important that these tools know they're working with a Cedar app. So this PR adds support for also configuring your app withcedar.toml. For nowcedar.tomlandredwood.tomlboth work exactly the same and have the same capabilities, but going forward they might start diverging with new features only being tested withcedar.toml.