search

mikebronner / laravel-model-caching

Eloquent model-caching made easy.
MIT License
2.23k stars 212 forks source link

Fix newBelongsToMany method compatibility #382

Closed dmason30 closed 3 years ago

dmason30 commented 3 years ago

Fixes #381

So this changes the method signature to use the Eloquent/Builder class directly in the method signature rather than the alias.

I created an example repo to replicate this issue here: https://github.com/dmason30/model-caching-example

This is the test used: https://github.com/dmason30/model-caching-example/blob/master/tests/Unit/ExampleTest.php

Before fix Replicated action: https://github.com/dmason30/model-caching-example/runs/1359149642?check_suite_focus=true

Shows PHP 7.3 Warning ![image](https://user-images.githubusercontent.com/20278756/98268419-67a5df00-1f84-11eb-8a1b-0e9f40f47bee.png)
Shows PHP 7.4 Warning ![image](https://user-images.githubusercontent.com/20278756/98268543-8f954280-1f84-11eb-826f-02167b872ea8.png)

After fix Example repo changed to use fork: https://github.com/dmason30/model-caching-example/pull/1/files Fix action: https://github.com/dmason30/model-caching-example/runs/1359345868?check_suite_focus=true

Shows PHP 7.3 No warnings ![image](https://user-images.githubusercontent.com/20278756/98268711-c5d2c200-1f84-11eb-87d4-7fd6c3ec50e0.png)
Shows PHP 7.4 No warnings ![image](https://user-images.githubusercontent.com/20278756/98268836-e7cc4480-1f84-11eb-9769-1a5220628f88.png)
gborcherds commented 3 years ago

Just came here to say this would be great to get merged in. :)

mikebronner commented 3 years ago

@dmason30 Thanks for submitting this! For some reason I seemed to have missed it completely. Sorry for the delay.