Skip to content

Switch app over to the 'modern' ways of auto-detecting middleware - #19560

Open
uberbrady wants to merge 1 commit into
grokability:developfrom
uberbrady:change_middleware_autoloading
Open

Switch app over to the 'modern' ways of auto-detecting middleware#19560
uberbrady wants to merge 1 commit into
grokability:developfrom
uberbrady:change_middleware_autoloading

Conversation

@uberbrady

Copy link
Copy Markdown
Member

So, this is allegedly the 'new' way that you're supposed to define your Middleware. I've been annoyed that, as we upgrade Laravel versions, we don't necessarily end up automagically getting the latest new Middleware stuff unless we add it in, by hand. For me, this just happened with the new Background Queue feature for Laravel. But I think @spencerrlongg got bit by this as well, when it came to something having to do with route-model-binding, maybe? So, I asked Claude to help me 'migrate' our middleware stacks and whatnot to whatever the 'modern' way is, and this is what it came up with.

It took a pretty conservative approach, which I think I'm mostly OK with - it notes its reasoning in the comments which all do make sense to me.

I have different changes to come that have to do with the Trusted Proxies headers, but those can be handled separately; this should be a true-ish translation of what-we-have-now into the 'modern' way, so that as we continue to upgrade Laravel versions, we should get whatever the newest middleware is supposed to be.

@uberbrady
uberbrady requested a review from snipe as a code owner August 26, 2026 18:18
@codacy-production

codacy-production Bot commented Aug 26, 2026

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 complexity

Metric Results
Complexity 0

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@snipe

snipe commented Aug 26, 2026

Copy link
Copy Markdown
Member

Okay, so, I hate this a lot (not your fault - this time), but I also understand that this is The (modern Laravel) Way™. The ONE thing I might ask, at least for a few versions until I get used to the new location, is that we leave Kernel.php in the source, BUT just make it an almost-empty file with a just a PHP comment that points to the new location of the middleware loaders, etc. 13+ years of looking for them in the same places means I will 100% keep looking for the Kernel file and keep not understanding why I can't find it.

I won't block a merge on just that, but it would be really helpful for me personally. I know this will end up biting me. :(

@uberbrady
uberbrady force-pushed the change_middleware_autoloading branch from e98f10a to d102b1a Compare August 27, 2026 20:42
@uberbrady

Copy link
Copy Markdown
Member Author

There's some weirdness in here - I'll get that file added for you, for sure, but I also feel like the Middleware::withHeaders() is conflicting with the overrriden middleware we wrote. I'm going to try and sort that out.

@uberbrady
uberbrady marked this pull request as draft August 27, 2026 20:57
@uberbrady
uberbrady force-pushed the change_middleware_autoloading branch from d102b1a to 239bf1a Compare August 31, 2026 15:35
@uberbrady
uberbrady marked this pull request as ready for review August 31, 2026 15:38
@uberbrady

Copy link
Copy Markdown
Member Author

Okay, I think I have this fixed up properly now - feel free to give it another look when you have a moment. It does pass tests.

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.

2 participants