-
-
Notifications
You must be signed in to change notification settings - Fork 784
fix: legacyDevProcessCwdBehaviour defaults to true #2387
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
This fixes an issue with prisma not being able to locate the native add-on binary
🦋 Changeset detectedLatest commit: 4427817 The changes in this PR will be included in the next version bump. 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 |
Walkthrough
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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 (3)
.changeset/afraid-waves-divide.md (1)
5-5
: Tighten the changeset summary wording and formatting.Clarify the intent and format identifiers as code for readability.
Apply this diff:
-Make the default of legacyDevProcessCwdBehaviour true instead of false (prevents breaking prismaExtension) +Make the default of `legacyDevProcessCwdBehaviour` true instead of false. This prevents breaking the Prisma native add-on resolution (e.g. the `prismaExtension`) in dev.packages/core/src/v3/config.ts (1)
286-288
: Avoid contradictory guidance: soften the “highly recommended” note or add context.Default is now true for compatibility (e.g., Prisma), but the current wording recommends false as “highly recommended,” which can confuse users. Recommend softening and adding context about native modules.
Apply this diff:
- * Setting this to `false` will set the current working directory to the build directory. - * This more closely matches the behavior of the CLI when running in production and is highly recommended. + * Setting this to `false` will set the current working directory to the build directory. + * This more closely matches the behavior of the CLI when running in production. + * Note: Some native modules (e.g. Prisma) may expect the project root as the CWD during development; in those cases keep this set to `true`.packages/cli-v3/src/dev/devSupervisor.ts (1)
361-367
: Prefer nullish coalescing for simpler defaulting.TypeScript already types legacyDevProcessCwdBehaviour as boolean | undefined. Using the nullish coalescing operator simplifies the logic without changing semantics.
Apply this diff:
- const legacyDevProcessCwdBehaviour = - typeof this.options.config.legacyDevProcessCwdBehaviour === "boolean" - ? this.options.config.legacyDevProcessCwdBehaviour - : true; + const legacyDevProcessCwdBehaviour = + this.options.config.legacyDevProcessCwdBehaviour ?? true;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/afraid-waves-divide.md
(1 hunks)packages/cli-v3/src/dev/devSupervisor.ts
(1 hunks)packages/core/src/v3/config.ts
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.{ts,tsx}
: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations
Files:
packages/cli-v3/src/dev/devSupervisor.ts
packages/core/src/v3/config.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
We use zod a lot in packages/core and in the webapp
Files:
packages/core/src/v3/config.ts
🧠 Learnings (5)
📚 Learning: 2025-07-18T17:50:25.014Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to trigger.config.ts : The `trigger.config.ts` file must use `defineConfig` from `trigger.dev/sdk/v3` and follow the configuration structure shown.
Applied to files:
packages/core/src/v3/config.ts
📚 Learning: 2025-07-18T17:50:25.014Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to trigger.config.ts : Build extensions such as `additionalFiles`, `additionalPackages`, `aptGet`, `emitDecoratorMetadata`, `prismaExtension`, `syncEnvVars`, `puppeteer`, `ffmpeg`, and `esbuildPlugin` must be configured in `trigger.config.ts` as shown.
Applied to files:
packages/core/src/v3/config.ts
📚 Learning: 2025-07-18T17:50:25.014Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : ALWAYS generate Trigger.dev tasks using the `task` function from `trigger.dev/sdk/v3` and export them as shown in the correct pattern.
Applied to files:
packages/core/src/v3/config.ts
📚 Learning: 2025-07-18T17:50:25.014Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to trigger.config.ts : Global lifecycle hooks, telemetry, runtime, machine settings, log level, max duration, and build configuration must be set in `trigger.config.ts` as shown.
Applied to files:
packages/core/src/v3/config.ts
📚 Learning: 2025-07-18T17:50:25.014Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : You MUST use `trigger.dev/sdk/v3` when writing Trigger.dev tasks.
Applied to files:
packages/core/src/v3/config.ts
🪛 LanguageTool
.changeset/afraid-waves-divide.md
[grammar] ~5-~5: Ensure spelling is correct
Context: ...er.dev": patch --- Make the default of legacyDevProcessCwdBehaviour true instead of false (prevents breakin...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
packages/core/src/v3/config.ts (1)
283-291
: Docs read cleanly; default annotation aligns with the new behavior.The JSDoc now reflects @default true and the behavior description is coherent with the runtime change in the CLI.
packages/cli-v3/src/dev/devSupervisor.ts (2)
361-367
: Default-to-true logic is correct and matches the new config default.Treating undefined/non-boolean as true and mapping to cwd=undefined (project CWD) vs build output path when false aligns with the intended behavior.
361-367
: DevRunControllercwd
handling is correct; please confirm externalBuildManifest.outputPath
is required
- DevRunControllerOptions (in
packages/cli-v3/src/entryPoints/dev-run-controller.ts
) declares
cwd?: string
and assignsthis.cwd = opts.cwd
.- In TaskRunProcess (spawn/execa calls), passing
cwd: undefined
falls back to Node’s default (parent CWD), soundefined
is handled as intended.BackgroundWorker
usesthis.build.outputPath
(from@trigger.dev/core/v3
) without guard—for safety, verify in your core v3 package thatBuildManifest.outputPath
is a mandatorystring
(notstring | undefined
).No changes required in this repository if
outputPath
is indeed non-optional.
This fixes an issue with prisma not being able to locate the native
add-on binary