feat(router): preload model relations during route binding

[xHeaven]

Closes #2110.

[github-actions]

Benchmark Results

Comparison of feature/with-relations-route-binding against 3.x (dae32bb2a607df2fac7c755a8f6344ea4bf8af84).

Open to see the benchmark results

No benchmark changes above ±5%.

_{Generated by phpbench against commit 3cd214f}

[brendt]

Looks good to me. One edge case I wonder about: \Tempest\Router\Bindable::resolve still has one parameter. That's fine, we don't break LSP by adding one more in the IsDatabaseModel trait. However, shouldn't we check whether the current resolve implementation within our RouteBindingInitializer actually accepts the relations?

I guess PHP will simply ignore the additional parameter if it's not present, so I guess we're good. Just wanted to double check if I hadn't missed anything

[xHeaven]

Looks good to me. One edge case I wonder about: \Tempest\Router\Bindable::resolve still has one parameter. That's fine, we don't break LSP by adding one more in the IsDatabaseModel trait. However, shouldn't we check whether the current resolve implementation within our RouteBindingInitializer actually accepts the relations?

I guess PHP will simply ignore the additional parameter if it's not present, so I guess we're good. Just wanted to double check if I hadn't missed anything

Good catch. Usually PHP would ignore the additional parameter. However, it's a named one, so that will throw. I'm not yet sure what we should do about this - making it unnamed feels a bit weird. What do you think would be good here?

[brendt]

Well… it would only call the method with named parameter if the attribute was provided.

Can we wrap this in an exception that clearly explains what's wrong? (eg. "You added preload to a class that doesn't use IsDatabaseModel").

It's a bit dirty, but I feel it's good enough for now

[brendt]

Perfect :)