feat(build-server): add option to specify pre-build command #2596
Conversation
Adds an option to specify a pre-build command in the build settings. Can be useful for projects that need a step before the build, e.g., to generate a prisma client. Also, remove the install directory in favor of simplicity. Both pre-build and install commands are run from the root of the repo. Users that need to run the commands in a different dir can just prepend to the command, e.g., `cd apps/web && pnpm run primsa:migrate`
|
WalkthroughReplaces the installDirectory field with preBuildCommand across build settings. The route file's form schema now exposes Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx (1)
1137-1137: Consider explicitly marking optional fields in labels.All three build setting fields (
triggerConfigFilePath,installCommand,preBuildCommand) are optional in the schema but the labels don't explicitly indicate this. Addingrequired={false}would make the optional nature more obvious to users.Example for the Pre-build command label:
- <Label htmlFor={fields.preBuildCommand.id}>Pre-build command</Label> + <Label htmlFor={fields.preBuildCommand.id} required={false}>Pre-build command</Label>Apply the same pattern to the other two labels for consistency.
Also applies to: 1158-1158, 1177-1177
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx(4 hunks)apps/webapp/app/v3/buildSettings.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{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:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsxapps/webapp/app/v3/buildSettings.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:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsxapps/webapp/app/v3/buildSettings.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
When importing from @trigger.dev/core in the webapp, never import the root package path; always use one of the documented subpath exports from @trigger.dev/core’s package.json
Files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsxapps/webapp/app/v3/buildSettings.ts
apps/webapp/app/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Modules intended for test consumption under apps/webapp/app/**/*.ts must not read environment variables; accept configuration via options instead
Files:
apps/webapp/app/v3/buildSettings.ts
🧬 Code graph analysis (1)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx (2)
apps/webapp/app/components/primitives/Hint.tsx (1)
Hint(3-9)apps/webapp/app/components/primitives/Label.tsx (1)
Label(25-41)
⏰ 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). (20)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: typecheck / typecheck
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
🔇 Additional comments (5)
apps/webapp/app/v3/buildSettings.ts (1)
3-7: LGTM! Schema update is clean and consistent.The addition of
preBuildCommandfollows the same pattern as the existinginstallCommandfield. Detailed validation (single-line, length limits) is appropriately handled at the form level in the route file, keeping this base schema simple and reusable.apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx (4)
159-178: Validation logic is well-structured.Both
installCommandandpreBuildCommanduse consistent validation:
- Single-line enforcement prevents newline-based command injection
- 500 character limit is reasonable for shell commands
- Field-specific error messages improve user experience
406-412: Action handler correctly processes the new field.The destructuring and payload construction properly handles
preBuildCommandalongside the existing fields, with appropriate empty-string-to-undefined conversion.
1104-1116: State management and change detection implemented correctly.The form state properly tracks
preBuildCommandalongside existing fields, with accurate change detection to enable/disable the save button.
1170-1173: Clear and helpful hint text.The updated hint for
installCommandprovides important context about execution location (repo root) and auto-detection behavior, helping users understand when they need to provide a custom command.
...p/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx (1)
1162-1162: Consider removing backticks from placeholder text.The placeholders include literal backticks (e.g.,
`npm install`), which are meant to indicate code but appear as regular characters in the input field. This might be slightly confusing for users.Consider using one of these alternatives:
Option 1 (preferred): Remove backticks entirely:
- placeholder="e.g., `npm install`, `pnpm install`, or `bun install`" + placeholder="e.g., npm install, pnpm install, or bun install"- placeholder="e.g., `npm run prisma:generate`" + placeholder="e.g., npm run prisma:generate"Option 2: Use single quotes instead:
- placeholder="e.g., `npm install`, `pnpm install`, or `bun install`" + placeholder="e.g., 'npm install', 'pnpm install', or 'bun install'"- placeholder="e.g., `npm run prisma:generate`" + placeholder="e.g., 'npm run prisma:generate'"Also applies to: 1181-1181
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx(4 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{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:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
{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:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
When importing from @trigger.dev/core in the webapp, never import the root package path; always use one of the documented subpath exports from @trigger.dev/core’s package.json
Files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
🧬 Code graph analysis (1)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx (2)
apps/webapp/app/components/primitives/Hint.tsx (1)
Hint(3-9)apps/webapp/app/components/primitives/Label.tsx (1)
Label(25-41)
⏰ 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). (22)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- 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 - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx (4)
159-178: LGTM! Validation logic is sound.The single-line validation for both
installCommandandpreBuildCommandprevents potential command injection via newlines, and the 500-character limit is reasonable for preventing abuse while allowing complex commands.
406-410: LGTM! Action handling correctly updated.The destructuring and payload construction properly handle the new
preBuildCommandfield, replacing the previousinstallDirectoryfield as intended.
1104-1116: LGTM! State management properly handles the new field.The state initialization and change detection for
preBuildCommandfollow the same pattern as other fields in the form, ensuring consistent behavior.
1189-1192: Grammar issue resolved.The hint text now correctly uses the singular form "Any command that needs to run" instead of the plural "commands" with singular verb "needs" that was flagged in the previous review. The singular form is actually more accurate since the field accepts a single command string.
Adds an option to specify a pre-build command in the build settings. Can
be useful for projects that need a step before the build, e.g., to
generate a prisma client.
Also, remove the install directory in favor of simplicity. Both
pre-build and install commands are run from the root of the repo. Users
that need to run the commands in a different dir can just prepend to the
command, e.g.,
cd apps/web && pnpm run primsa:migrate