Skip to content

Conversation

liamduckett
Copy link
Contributor

Hey, I believe the current implementation of generics on the QueryBuilder don't work as expected.

Currently, the following code:

dumpType(QueryBuilder::for(User::class));
dumpType(QueryBuilder::for(Auth::user()->categories()));
dumpType(QueryBuilder::for(User::query()));

Produces the following:

Dumped type: Spatie\QueryBuilder\QueryBuilder<Illuminate\Database\Eloquent\Model>                                                                                           
Dumped type: Spatie\QueryBuilder\QueryBuilder<Illuminate\Database\Eloquent\Model>                                                                                           
Dumped type: Spatie\QueryBuilder\QueryBuilder<Illuminate\Database\Eloquent\Model>

In order to get this to the more precise:

Dumped type: Spatie\QueryBuilder\QueryBuilder<App\Models\User>                                                                                           
Dumped type: Spatie\QueryBuilder\QueryBuilder<App\Models\Category>                                                                                           
Dumped type: Spatie\QueryBuilder\QueryBuilder<App\Models\User>

I have implemented two changes. The first, is to use a new generic for the for method, as it is static. See examples: using generic from class & using generic from method .

The second is to specify make use of this generic, when calling the method with a Relation. The <T, *, *> syntax means we're making use of the first generic, but don't care about the second and third (they can be anything - hence the asterisk).

I think ideally there would some type assertions for this functionality - I'd be open to adding some if desirable?

@freekmurze
Copy link
Member

@liamduckett Thank you for this, please do add some type assertions 👍

@BertvanHoekelen
Copy link

Thanks @liamduckett ! Just ran into the same issue myself, saves me from opening a PR :)

@liamduckett
Copy link
Contributor Author

@freekmurze I've added a new folder types that gets analysed whenever PHPStan is ran. Does this feel suitable?

@freekmurze freekmurze merged commit d6c1373 into spatie:main Jul 25, 2025
1 check passed
@freekmurze
Copy link
Member

Very nice, thank you!

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.

3 participants