feat(webapp): make native build server the default in build settings#3980
feat(webapp): make native build server the default in build settings#3980myftija wants to merge 1 commit into
Conversation
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📜 Recent review details⏰ 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). (13)
WalkthroughThe native build server setting is inverted from an opt-in flag ( 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Switch the native build server from opt-in to opt-out. It's now enabled by default and stored as a new `disableNativeBuildServer` opt-out key, so previously-saved `useNativeBuildServer: false` values aren't mistaken for deliberate opt-outs. Clarify in the UI that build settings apply to GitHub-triggered and native build server deployments.
964d07e to
49dd55e
Compare
There was a problem hiding this comment.
🚩 No server-side consumers of the native build server flag found
Searched the entire codebase for consumers of useNativeBuildServer or disableNativeBuildServer outside the settings UI route. Found zero usages in deployment services, GitHub-triggered deploy flows, or any backend build logic. The flag appears to be stored in the database but only read back by the settings UI presenter (apps/webapp/app/services/projectSettingsPresenter.server.ts:31). The CLI's nativeBuildServer option in packages/cli-v3/src/commands/deploy.ts:83 is a separate CLI flag (z.boolean().default(false)) that doesn't read from the server's buildSettings. This means the schema rename has no backend behavioral impact — it's purely a UI/storage concern. However, this raises the question: is the disableNativeBuildServer flag actually consumed by the GitHub-triggered deployment flow to decide whether to use the native build server? If not, this setting may be a no-op for that deployment path.
Was this helpful? React with 👍 or 👎 to provide feedback.
| // Opt-out flag: the native build server is used by default. Only set when a | ||
| // project explicitly disables it. Absence means native build server enabled. | ||
| disableNativeBuildServer: z.boolean().optional(), |
There was a problem hiding this comment.
🚩 Implicit data migration via Zod key stripping — old useNativeBuildServer values silently dropped
Existing database records may contain { useNativeBuildServer: true } or { useNativeBuildServer: false } in the buildSettings JSON column. After the schema rename to disableNativeBuildServer at apps/webapp/app/v3/buildSettings.ts:9, Zod's default .object() behavior strips unrecognized keys during safeParse at apps/webapp/app/services/projectSettingsPresenter.server.ts:31. This means:
- Projects with
useNativeBuildServer: true→ parsed as{}→disableNativeBuildServerabsent → treated as enabled ✓ - Projects with
useNativeBuildServer: false→ parsed as{}→disableNativeBuildServerabsent → treated as enabled (was previously disabled) - Projects with no
buildSettingsat all → treated as enabled (was previously disabled)
This is the intended behavior per the PR description ("make native build server the default"), but it's worth noting that there is no explicit data migration — the behavior change happens implicitly through the schema rename. Projects that previously had native build server explicitly disabled will have it silently re-enabled. This is presumably acceptable since the PR is intentionally flipping the default, but reviewers should confirm this is the desired rollout strategy.
Was this helpful? React with 👍 or 👎 to provide feedback.
Switches the native build server from opt-in to opt-out in project build settings.