fix(studio): honor MAINTENANCE_MODE in the TanStack runtime - #48616
Conversation
The root route's beforeLoad called matchRedirect without maintenanceMode, so it fell back to the '= false' default and the TanStack runtime never applied maintenance mode: no path redirected to /maintenance, and the 'not in maintenance' branch sent /maintenance to /, leaving routes/maintenance.tsx unreachable. MAINTENANCE_MODE is unprefixed, but it is already a build-time var in both other consumers - Next bakes redirects() into routes-manifest.json during next build, and vercel.ts reads it while emitting vercel.json. Inline it in vite.config.ts on the same terms so the isomorphic beforeLoad can read it on the client too. No new env var, and no deployment-config change for self-hosters. Also adds unit tests for the maintenance branches of matchRedirect, which had no coverage. Fixes supabase#48559 Co-Authored-By:...>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
6 Skipped Deployments
|
|
Thanks for contributing to Supabase! ❤️ Our team will review your PR. A few tips for a smoother review process:
|
|
@binitadkl is attempting to deploy a commit to the Supabase Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughStudio now injects ChangesMaintenance mode redirect flow
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This localized change makes the TanStack runtime honor the existing maintenance-mode setting while preserving normal routing when it is unset. No actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Description checkExplanation The description covers the change type, current behavior, linked issue, new behavior, implementation details, validation, and additional context. It omits the required CONTRIBUTING.md confirmation section, but the remaining description is complete. Full details: Linked Issues checkExplanation The changes address issue Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 3 files. ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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.
🧹 Nitpick comments (1)
apps/studio/redirects.shared.test.ts (1)
147-205: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test for the root-route wiring.
These tests call
matchRedirectdirectly. They would still pass ifapps/studio/routes/__root.tsxstopped passingmaintenanceModeat Line [338]. Add a focused test forRoute.beforeLoador the existing route harness.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/studio/redirects.shared.test.ts` around lines 147 - 205, Add a focused test covering the root route’s beforeLoad wiring, using Route.beforeLoad or the existing route harness to verify it passes the maintenance-mode value into matchRedirect. Keep the existing direct matchRedirect tests unchanged and assert the redirect behavior through the route integration path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/studio/redirects.shared.test.ts`:
- Around line 147-205: Add a focused test covering the root route’s beforeLoad
wiring, using Route.beforeLoad or the existing route harness to verify it passes
the maintenance-mode value into matchRedirect. Keep the existing direct
matchRedirect tests unchanged and assert the redirect behavior through the route
integration path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3747afd3-b3ec-4cc3-9bb0-5c758a8e2aa8
📒 Files selected for processing (3)
apps/studio/redirects.shared.test.tsapps/studio/routes/__root.tsxapps/studio/vite.config.ts
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
alaister
left a comment
alaister
left a comment
There was a problem hiding this comment.
Thanks for the contribution @binitadkl! Really solid PR – the design note and verification tables made this an easy review. I merged in the latest master and re-verified the maintenance build end to end (server redirects, client bundle, /img allowlist, query preservation) and it all works as described 🙌
Braintrust eval report
|
2 participants
What kind of change does this PR introduce?
Bug fix.
What is the current behavior?
Fixes #48559 (diagnosed by @ayaangazali)
The TanStack Start runtime never applies maintenance mode.
matchRedirectinapps/studio/redirects.shared.tstakes amaintenanceModeflag, and both other consumers wire it from the environment:
apps/studio/next.config.ts—process.env.MAINTENANCE_MODE === 'true'apps/studio/vercel.ts— sameThe TanStack call site in
apps/studio/routes/__root.tsxpassed onlypathname,search,isPlatformandhash, somaintenanceModefell backto its
= falsedefault. WithMAINTENANCE_MODE=trueon a TanStack deploythat produced two wrong behaviors:
/maintenance— the app served normally duringmaintenance.
and sent
/maintenance→/, makingroutes/maintenance.tsxunreachable.Mainly affects self-hosted / Node-server TanStack deploys; the platform deploy
is covered by the Vercel edge layer, which does wire the flag.
What is the new behavior?
The TanStack runtime honors
MAINTENANCE_MODEthe same way the Next runtimeand the edge config do.
Design note. The issue asked whether this needs a new
NEXT_PUBLIC_variable or server-side plumbing, since both would change deployment
configuration for self-hosters. Neither is needed.
MAINTENANCE_MODEisalready a build-time variable in both existing consumers — Next bakes
redirects()intoroutes-manifest.jsonduringnext build, andvercel.tsreads it while emitting
vercel.json. Toggling maintenance has always requireda rebuild, never just a server restart. And
vite.config.tsisn't bound byNext's "only
NEXT_PUBLIC_" rule: it controlsdefinedirectly, and alreadyre-exposes unprefixed
VERCEL_*vars the same way. So the existing unprefixedvariable is inlined at build time, giving exact parity with no new env var
and no config change for self-hosters.
Three changes:
vite.config.ts— inlineprocess.env.MAINTENANCE_MODEinto the bundle.Falls back to
''rather than being left undefined, so the browser bundlenever ends up with a bare
process.envreference (the failure mode the filealready guards against for the Sentry vars).
routes/__root.tsx— read it intoIS_MAINTENANCE_MODEand pass it tomatchRedirect.redirects.shared.test.ts— 4 tests for the maintenance branches ofmatchRedirect, which had no coverage at all.turbo.jsoncalready listsMAINTENANCE_MODEunder the build task'senv, socache invalidation is correct for the Vite build too — no change needed. No
README or docs change either, since the env contract is unchanged.
Additional context
Verified end-to-end, not just by unit test.
Browser repro — built SPA served via
scripts/serve.js, driven in headlessChromium:
MAINTENANCE_MODE=true/project/default/maintenance//maintenance/maintenance/maintenanceThe maintenance page renders real content ("Under Maintenance — We are
currently improving our services…"), so the route is genuinely reachable.
/project/default/project/default//project/default/maintenance/project/defaultBundle inspection — the flag compiles to a literal
truewith the variableset and
falsewithout it, confirming the define reaches the client.Shell prerender — checked explicitly, since the maintenance-on rule is a
catch-all. Builds with
MAINTENANCE_MODE=trueprerender the SPA shell and passthe post-build smoke test; the prerenderer crawls
/and the rootbeforeLoadredirect does not fire during shell generation, so no guard is required.
Checks — 20 unit tests pass, typecheck 8/8, ESLint ratchet passes, Prettier
clean.
Summary by CodeRabbit
New Features
Tests