route:after should not be called on $route→next()

[distantnative]

I just encountered the problem while debugging Retour (distantnative/retour-for-kirby#138) that the route:after hook is a bit problematic in multilang setups.

I would argue the most common use case to hook into a successful routing.
However, the hook is actually called on every route lookup - also if the route does $this→next(), which seems counter intuitive as the first route explicitly says "not me" by calling $this→next()
https://github.com/getkirby/kirby/blob/master/src/Http/Router.php#L115-L117

My suggestion would be to move the hook call out of the loop and right before the return. To really only call the hook on routes that do not skip to the next route.
What do you think @bastianallgeier and @lukasbestle?

[lukasbestle]

I agree that this is a bug in the current implementation.

The issue is that the before hook is called on every route and there is no way to only call it on the route that will be used in the end. So if we do it like you suggest, there will be an asymmetry between the before hook (called for every route) and the after hook (called only on the successful route).

I see two solutions to keep this consistent:

• We continue to call the after hook on every route, but pass another boolean argument that tells the hook whether the route was used or not.
• We also only run the before hook once and don't pass the $route variable to it (breaking-change!).

[distantnative]

I like your first suggestion the most.

[bastianallgeier]

I like the first suggestion as well. It also means we can implement it without a breaking change!

[bastianallgeier]

✅