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

There was a mistake in the new commit #455

Open
Dmytro-Kostetskyi opened this issue Jan 3, 2025 · 6 comments
Open

There was a mistake in the new commit #455

Dmytro-Kostetskyi opened this issue Jan 3, 2025 · 6 comments

Comments

@Dmytro-Kostetskyi
Copy link

[2025-01-03 16:45:51] development.ERROR: Declaration of Ebess\AdvancedNovaMediaLibrary\Fields\Media::resolve($resource, ?string $attribute = null): void must be compatible with Laravel\Nova\Fields\Field::resolve($resource, $attribute = null) {"userId":1,"exception":"[object] (Symfony\Component\ErrorHandler\Error\FatalError(code: 0): Declaration of Ebess\AdvancedNovaMediaLibrary\Fields\Media::resolve($resource, ?string $attribute = null): void must be compatible with Laravel\Nova\Fields\Field::resolve($resource, $attribute = null) at /vendor/ebess/advanced-nova-media-library/src/Fields/Media.php:290)
[stacktrace]
#0 {main}
"}

    "laravel/framework": "^11.0",
    "laravel/nova": "4.33.0",
@kiritokatklian
Copy link
Contributor

kiritokatklian commented Jan 4, 2025

@bkintanar ping! The changes in 4.3 (#454) are actually incompatible with Nova 4.x contrary to the requirements in the composer file
I'd suggest reverting the changes in 4.3, then removing Nova 4.0 support and bumping this library to 5.0 since it's a breaking change

@gerasart
Copy link

gerasart commented Jan 5, 2025

got the same - where unit tests?

@bkintanar
Copy link
Collaborator

@kiritokatklian i've reverted the changes, and merged the changes necessary for Nova 5.0 support. I currently do not have access to Nova anymore, can someone test the latest commit in master?

@kiritokatklian
Copy link
Contributor

kiritokatklian commented Jan 5, 2025

@bkintanar #454 contained two breaking changes to support Nova 5. Your commit only reverts one of the changes, namely the one in Composer.

The other change is Ebess\AdvancedNovaMediaLibrary\Fields\Media::resolve method's signature changed in Nova 5 to include parameter types, but Nova 4 doesn't have that leading to the following error due to the mismatch:

Declaration of Ebess\AdvancedNovaMediaLibrary\Fields\Media::resolve($resource, ?string $attribute = null): void must be compatible with Laravel\Nova\Fields\Field::resolve($resource, $attribute = null)

I tested locally by fixing the method's signature and the changes in #456 are working with Nova 4. I opened #457 to fix the issue above.

I'd first release #456 and #457 together to fix the issue for Nova 4 users once and for all.
After that it should be fine to re-implement the changes in #454 (and drop Nova 4 support) then release a 5.0.0 for Nova 5 users.

kiritokatklian referenced this issue Jan 5, 2025
@bkintanar
Copy link
Collaborator

@kiritokatklian thanks! I've done the above, released both 4.4 (Nova 4 support) and 5.0 (Nova 5 support). Can you or anyone test it please?

Thank you

@kiritokatklian
Copy link
Contributor

kiritokatklian commented Jan 5, 2025

@bkintanar amazing, thanks! Both versions working as expected here 😄

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

4 participants