Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: update Eloquent Builder stubs to be more specific #1178

Merged
merged 1 commit into from
Mar 18, 2022

Conversation

canvural
Copy link
Collaborator

  • Added or updated tests
  • Documented user facing changes
  • Updated CHANGELOG.md

Resolves #1173

Changes

This PR updates Eloquent builder method stubs to be more specific. So the correct builder will be inferred inside the callbacks.

Breaking changes
none

@canvural canvural marked this pull request as draft March 16, 2022 14:58
@canvural canvural force-pushed the update-eloquent-builder-stubs branch 2 times, most recently from 3f958e1 to 7562f92 Compare March 17, 2022 10:03
@canvural canvural marked this pull request as ready for review March 17, 2022 10:03
@canvural canvural marked this pull request as draft March 17, 2022 10:09
@fragkp
Copy link
Contributor

fragkp commented Mar 18, 2022

Hey @canvural!
Thanks for your work on this 👍

I've tested your branch in our project and here are the results:

UserChallenge::whereHas('challenge', fn (ChallengeBuilder) ...);

PHPStan Error:

expects (Closure(App\Models\Builders\UserChallengeBuilder): void)|(Closure(App\Models\Builders\UserChallengeBuilder): App\Models\Builders\UserChallengeBuilder)|null, 
Closure(App\Models\Builders\ChallengeBuilder): App\Models\Builders\ChallengeBuilder given.

Problem:
whereHas expects a custom eloquent builder of the given relation.


$this->hasOne(CompanyGoal::class)->where(fn (CompanyGoalBuilder $query) => ...);

PHPStan Error:

Parameter #1 $column of method Illuminate\Database\Eloquent\Relations\HasOne<App\Models\CompanyGoal>::where() expects
  array<int|string, mixed>|(Closure(Illuminate\Database\Eloquent\Relations\HasOne<App\Models\CompanyGoal>): void)|(Closure(Illuminate\Database\Eloquent\Relations\HasOne<App\Models\CompanyGoal>):    Illuminate\Database\Eloquent\Relations\HasOne<App\Models\CompanyGoal>)|Illuminate\Database\Query\Expression|string,
  Closure(App\Models\Builders\CompanyGoalBuilder): App\Models\Builders\CompanyGoalBuilder given.

Problem:
relation methods aren't allowed to use custom eloquent builders.

@canvural
Copy link
Collaborator Author

@fragkp Yes I know about that problem. That's why the PR is still in draft. And look like that problem will not be solved. PHPStan does not have a way to support it currently. So I'll revert the changes to relation existence methods.

@canvural canvural force-pushed the update-eloquent-builder-stubs branch from 7562f92 to 140dd60 Compare March 18, 2022 08:35
@canvural canvural marked this pull request as ready for review March 18, 2022 08:57
@canvural canvural merged commit eef292f into master Mar 18, 2022
@canvural canvural deleted the update-eloquent-builder-stubs branch March 18, 2022 09:49
@mabdullahsari
Copy link

@canvural Looks like this did the trick. Eyvallah.

@szepeviktor
Copy link
Collaborator

Eyvallah.

The Turkish "OK".

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.

Custom eloquent builders won't work with where & orWhere since 2.1.0
4 participants