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

Compatibility with Laravel 8 #26

Closed
mjainta opened this issue Oct 15, 2020 · 3 comments
Closed

Compatibility with Laravel 8 #26

mjainta opened this issue Oct 15, 2020 · 3 comments

Comments

@mjainta
Copy link

mjainta commented Oct 15, 2020

Hello there,

I wonder if there is any issue with laravel-upsert and Laravel 8 right now, since I ran into this issue while upgrading from 7 to 8:
Declaration of Staudenmeir\LaravelUpsert\Query\Builder::upsert(array $values, $uniqueBy, ?array $update = NULL) should be compatible with Illuminate\Database\Query\Builder::upsert(array $values, $uniqueBy, $update = NULL)

For me it looks like that the definitions just dont match:
`
// laravel-upsert/src/Query/Builder.php

/**
 * Insert new records or update the existing ones.
 *
 * @param array $values
 * @param array|string $target
 * @param array|null $update
 * @return int
 */
public function upsert(array $values, $uniqueBy, array $update = null)`

`
// laravel/framework/src/Illuminate/Database/Query/Builder.php

/**
 * Insert new records or update the existing ones.
 *
 * @param  array  $values
 * @param  array|string  $uniqueBy
 * @param  array|null  $update
 * @return int
 */
public function upsert(array $values, $uniqueBy, $update = null)`

The problem is the "array" type definition it seems.
I see that there is a commit to upgrade to Laravel 8, so my question is if this is an issue on my particular project or it is missing in laravel-upsert?

If you would like to have a PR from me, I could help out.

Best regards, Martin

@einarjohanhansen
Copy link

The same happened to me, when updating laravel/framework from v8.9.0 to v8.10.0. Seems like Laravel now has this feature - https://github.com/laravel/framework/releases/tag/v8.10.0

@staudenmeir
Copy link
Owner

Laravel 8.10 integrated the package's code but modified the method signature.

I recommend that you remove or disable the package and try if the Laravel implementation behaves the same way as the package.

@mjainta
Copy link
Author

mjainta commented Oct 16, 2020

Thank you for the fast response.
I could use Laravel's integrated method.
Still, thanks for that awesome feature which made it to the main repo. 👍

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

No branches or pull requests

3 participants