Skip to content

chore: upgrade to Laravel 13#4

Merged
than merged 1 commit into
mainfrom
chore/laravel-13-upgrade
May 28, 2026
Merged

chore: upgrade to Laravel 13#4
than merged 1 commit into
mainfrom
chore/laravel-13-upgrade

Conversation

@than

@than than commented May 28, 2026

Copy link
Copy Markdown
Owner

Summary

Major framework upgrade: Laravel 12 → 13, Tinker 2 → 3, Boost 1 → 2. Stacked on top of #3 — review/merge that one first (this PR's base is the modernize branch, so the diff shows only the L13 changes).

What changed

  • laravel/framework ^12^13 (13.12.0), laravel/tinker ^2^3, laravel/boost ^1^2.
  • Composer resolved the rest of the ecosystem to L13-compatible versions automatically — no constraint changes needed: Pulse 1.7, Sanctum 4.3, Nightwatch 1.27, Collision 8.9, Pest 4.7, PHPUnit 12.5, Livewire 3.
  • Updated the renamed CSRF middleware to PreventRequestForgery (routes/web.php withoutMiddleware, config/sanctum.php). The VerifyCsrfToken/ValidateCsrfToken aliases still exist but are deprecated.

Breaking-change audit (Laravel 13 upgrade guide)

Checked each item against this app:

  • CSRF rename — updated (above). ✅
  • upsert uniqueBy validation — app has no upsert calls. ✅
  • serializable_classes cache guard — the only cached value (GlobalStats) returns pure arrays, never objects. No config needed. ✅
  • Cache prefix / session cookie hyphens — config files already use the new hyphen format, so no cache-key churn or session reset on deploy. ✅
  • Model boot() instance creationPlay::booted() only registers a creating listener; doesn't create instances. ✅
  • Queue events, custom managers, pagination views, polymorphic pivots — not used. ✅

Verification

  • App boots: Laravel Framework 13.12.0
  • php artisan test28/28 passing
  • php artisan config:clear / route:list clean
  • Pint clean

Deploy notes

🤖 Generated with Claude Code

@than than changed the base branch from chore/modernize-deps-and-pnpm to main May 28, 2026 15:47
- Bump laravel/framework ^12 -> ^13, laravel/tinker ^2 -> ^3,
  laravel/boost ^1 -> ^2; composer resolved Pulse, Sanctum, Nightwatch,
  Collision, Pest, etc. to L13-compatible versions automatically
- Update the renamed CSRF middleware references to PreventRequestForgery
  (routes/web.php withoutMiddleware, config/sanctum.php) — the old
  VerifyCsrfToken/ValidateCsrfToken aliases still exist but are deprecated

Verified: app boots on Laravel 13.12.0, 28/28 tests passing, Pint clean.
No migrations, no env changes. cache/session prefixes already use the
L13 hyphen format, and the app caches only arrays (serializable_classes
guard is a no-op).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@than than force-pushed the chore/laravel-13-upgrade branch from 25851f6 to f8995a6 Compare May 28, 2026 15:49
@claude

claude Bot commented May 28, 2026

Copy link
Copy Markdown

Review — Laravel 13 upgrade

Reviewed as a senior Laravel engineer. This is a clean, low-risk, well-documented dependency bump. No blockers. The functional surface is tiny (a CSRF middleware rename in two files); the rest is composer.lock churn. I verified the audit claims rather than taking them on faith, and they hold up.

What I verified independently

  • Constraint changes are minimal and correct. framework ^12→^13, tinker ^2→^3, boost ^1→^2. First-party deps were left at their existing constraints, which already admit L13-compatible releases — the lock resolved Pulse v1.7.3, Sanctum v4.3.2, Nightwatch v1.27.0, Boost v2.4.8. Avoiding needless constraint churn is the right call here.
  • CSRF rename is consistent. PreventRequestForgery is imported and used in both routes/web.php:73 and config/sanctum.php:84, and grep finds no lingering VerifyCsrfToken/ValidateCsrfToken references anywhere in app/, config/, routes/, or bootstrap/. There's no custom App\Http\Middleware\VerifyCsrfToken to break (correct for the L11+ structure).
  • Audit spot-checks pass. Play::booted() (app/Models/Play.php:25) only registers a creating UUID listener — no model instantiation. The only cache write in the app is global-stats (app/Livewire/Stats/GlobalStats.php:14), and it returns a plain scalar array — so the L13 serializable_classes guard really is a no-op here.
  • The critical server-authoritative behavior stays covered. tests/Feature/ApiPlayTest.php includes it('ignores a lying client and records the server-computed pattern', ...) asserting a client lie (NO_WIN/0) is overridden by the server-computed THREE_PAIR/500. That property is exercised under the new framework, which is exactly what matters for an upgrade PR.

Minor notes (none blocking)

a. PR description says "Livewire 3" but the lock pins Livewire v4.3.0. Livewire isn't a direct dependency (it comes in transitively), and the diff doesn't touch it — so this is not a regression introduced here; it was already v4 on main. But the body's claim is inaccurate, which slightly undercuts trust in an otherwise careful audit. Worth correcting. (Side note: the project's CLAUDE.md foundation context also still says "Livewire 3" — stale, out of scope for this PR but worth a future cleanup.)

b. Symfony 7.4 → 8.0 rides along transitively. The lock bumps the symfony/* components across a major (v7.4.x → v8.0.x), which is expected for L13. The audit enumerates Laravel-level breaking changes but doesn't mention the underlying Symfony major. Low risk for this app (no low-level HttpKernel/console customization), but it's a bigger underlying change than the items listed, so worth acknowledging in the writeup.

c. withoutMiddleware([PreventRequestForgery::class]) on the api group is effectively a no-op. The api middleware group doesn't carry CSRF (that lives in web), so there's nothing to strip. This predates the PR (it was VerifyCsrfToken::class before), so it's out of scope — but note that ::class resolves to a string without autoloading or validating the class, so a wrong FQCN here would silently fail to strip rather than error. It's harmless today precisely because the group has no CSRF to remove; just be aware the rename's correctness isn't actually load-bearing or test-exercised.

d. One transitive patch downgrade (v1.38.1 → v1.38.0) appears in the lock — composer picked a slightly older patch under the new resolution. Harmless, just flagging for awareness.

e. Couldn't run the suite here / "28/28" looks understated. The review environment has no vendor/, so I'm relying on your reported 28/28 passing. For what it's worth, the repo currently has ~60 it()/test() definitions (PatternDetector 27 + ApiPlay 17 alone = 44), so the 28/28 figure reads as stale or a subset — worth confirming it reflects a full current run before deploy.

Stacking

Body notes this is stacked on the modernize PR ("review/merge that one first"); the diff here is correctly isolated to the L13 changes (4 files), so it reviews cleanly in isolation. Land the base PR first as instructed.

Nice, disciplined upgrade. Fix the description nits (a/b/e) and this is good to merge.

@than than merged commit 06f445d into main May 28, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant