-
-
Notifications
You must be signed in to change notification settings - Fork 706
Auto-fix config.dirs of /trigger and /src/trigger #1665
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
Conversation
🦋 Changeset detectedLatest commit: 0421b9c The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
packages/cli-v3/src/config.tsOops! Something went wrong! :( ESLint: 8.45.0 ESLint couldn't find the config "custom" to extend from. Please check that the name of the config is correct. The config "custom" was referenced from the config file in "/.eslintrc.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. WalkthroughThis pull request introduces changes that improve the configuration handling for the development CLI. A new changeset document adds an auto-fix mechanism for resolving trigger directories as relative paths. Additionally, a new VS Code launch configuration is added for debugging the dev CLI, and modifications to the CLI configuration logic include a new function to ensure unique and correctly resolved directory paths. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant VS as VS Code
participant CLI as Dev CLI
participant RC as resolveConfig
participant RT as resolveTriggerDir
Dev->>VS: Launch "Debug V3 init dev CLI"
VS->>CLI: Execute "pnpm exec trigger dev" (cwd: init-shell)
CLI->>RC: Load configuration
RC->>RT: Process directory paths for /trigger and /src/trigger
RT-->>RC: Return adjusted relative paths
RC-->>CLI: Return unique configuration
CLI-->>VS: Start debugging session
Possibly related PRs
Poem
Tip 🌐 Web search-backed reviews and chat
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/cli-v3/src/config.ts (1)
195-206
: Consider enhancing the function's readability.The function logic is correct, but could be improved:
function resolveTriggerDir(dir: string, workingDir: string): string { if (isAbsolute(dir)) { - // If dir is `/trigger` or `/src/trigger`, we should add a `.` to make it relative to the working directory + // Special case: Convert root-level trigger directories to be relative to working directory + // This prevents misconfiguration when users specify absolute paths like `/trigger` or `/src/trigger` if (dir === "/trigger" || dir === "/src/trigger") { return `.${dir}`; - } else { - return relative(workingDir, dir); } + return relative(workingDir, dir); } return dir; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/lovely-toys-obey.md
(1 hunks).vscode/launch.json
(1 hunks)packages/cli-v3/src/config.ts
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: units / 🧪 Unit Tests
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
packages/cli-v3/src/config.ts (2)
158-158
: LGTM! Good improvements to directory handling.The changes effectively:
- Ensure consistent handling of trigger directories
- Prevent duplicate directories in the final configuration
Also applies to: 162-162, 190-190
158-190
: Verify impact on existing configurations.Let's ensure these changes don't affect existing valid configurations.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for any trigger.config.ts files and their directory configurations echo "Searching for trigger.config.ts files..." fd "trigger\.config\.ts$" --exec rg -A 5 "dirs:|triggerDirectories:" {}Length of output: 863
No impact on valid configurations detected.
The shell script output confirms that trigger configuration files continue to define valid directory paths (e.g.
["./src/trigger"]
or["./src"]
), and our modifications inpackages/cli-v3/src/config.ts
—mapping each directory viaresolveTriggerDir
and deduplicating them—preserve the intended behavior without introducing misconfiguration.
- The processing of
dirs
resolves them relative to the working directory.- Existing trigger configuration files remain correct and produce expected directory values.
.changeset/lovely-toys-obey.md (1)
1-6
: LGTM! Clear and concise changeset.The changeset accurately describes the purpose and impact of the changes.
.vscode/launch.json (1)
49-56
: Verify if this new configuration is necessary.There appears to be overlap with the existing "Debug V3 Dev CLI" configuration at line 60. The main difference is the working directory. Please confirm if both configurations are needed.
Could you clarify the specific use case that requires a separate configuration for
references/init-shell
?
@trigger.dev/build
trigger.dev
@trigger.dev/react-hooks
@trigger.dev/rsc
@trigger.dev/core
@trigger.dev/sdk
commit: |
Summary by CodeRabbit