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

imagine_filter fail when lazy mode is enabled #4808

Merged
merged 1 commit into from
Nov 17, 2021

Conversation

Yoann-TYT
Copy link
Contributor

@Yoann-TYT Yoann-TYT commented Nov 16, 2021

Hi everybody !

The problem occurs when I use the VichImageType and its form widget associated.
This line ea_apply_filter_if_exists('imagine_filter', formTypeOptions.imagine_pattern) fail.

I got the error Non-static method Liip\ImagineBundle\Templating\LazyFilterRuntime::filter() cannot be called statically when liip_imagine is configurered in lazy mode

# liip_imagine.yaml
liip_imagine:
    twig:
        mode: 'lazy'

Everything works very well if this mode is disabled.

You can reproduce that problem with the current version of Symfony ( 5.3 ) with php 8.

Just a more complex problem, getFilter is marked as internal by Twig twigphp/Twig#1889
So, we shouldn't use it... but I don't see how to avoid it :-/

I also changed the strict comparison, because on version 2 getFilter can return false but on version 3 it can return null.

Thanks a lot for your review :)

@Yoann-TYT Yoann-TYT changed the title Fix lazy filter imagine ea_apply_filter_if_exists('imagine_filter', formTypeOptions.imagine_pattern) fail when lazy mode is enabled Nov 16, 2021
@Yoann-TYT Yoann-TYT changed the title ea_apply_filter_if_exists('imagine_filter', formTypeOptions.imagine_pattern) fail when lazy mode is enabled imagine_filter fail when lazy mode is enabled Nov 16, 2021
@javiereguiluz javiereguiluz added this to the 3.x milestone Nov 17, 2021
@javiereguiluz javiereguiluz merged commit d95d5e2 into EasyCorp:master Nov 17, 2021
@javiereguiluz
Copy link
Collaborator

Yoann, thanks for this fantastic bug fix and for the perfect explanation!

While merging I did a minor tweak (4eea6ae) because I prefer to always use strict comparisons. Cheers!

@Yoann-TYT
Copy link
Contributor Author

I also prefer to use strict comparisons. Thanks for your great reactivity !

@asispts asispts mentioned this pull request Jun 6, 2023
6 tasks
javiereguiluz added a commit that referenced this pull request Jun 22, 2023
This PR was squashed before being merged into the 4.x branch.

Discussion
----------

[Chore] Fix phpstan issues

This PR will fix several `PHPStan` issues:
  - [x] Fix `Cannot unset offset string on array<int, EasyCorp\Bundle\EasyAdminBundle\Dto\ActionDto>.`
  - [x] Fix `Variable method call on Twig\Extension\RuntimeExtensionInterface.`
      - [x] Refactor `EasyAdminTwigExtension::applyFilterIfExists`
      - [x] Add integration tests for `applyFilterIfExists`. Issues #3762 and #4808 have been taken into consideration.
  - [x] Update parameter type declaration of `Action::new`. We can't use ``@var`` to annotate a parameter. See: https://phpstan.org/r/7695e922-db7f-42b3-ac4f-8ff65a8a37b9
  - [x] Modify incorrect PHPDoc. See: https://phpstan.org/r/df4eb1c2-7286-4aa7-b276-f36ff2c0c82c

These issues are a little bit tricky. Both `TranslatableMessageBuilder::withParameters` and `TranslatableChoiceMessage::__toString` are not compatible with `TranslatableInterface`. Those functions should depend on `TranslatableMessage`.
```txt
 ------ -----------------------------------------------------------------------
 Line   Field/Configurator/ChoiceConfigurator.php
 ------ -----------------------------------------------------------------------
  124    Parameter #1 $message of class
         EasyCorp\Bundle\EasyAdminBundle\Translation\TranslatableChoiceMessage
         constructor expects
         Symfony\Component\Translation\TranslatableMessage,
         Symfony\Contracts\Translation\TranslatableInterface given.
 ------ -----------------------------------------------------------------------

 ------ -----------------------------------------------------------------------
 Line   Translation/TranslatableMessageBuilder.php
 ------ -----------------------------------------------------------------------
  28     Call to an undefined method
         Symfony\Contracts\Translation\TranslatableInterface::getMessage().
  29     Call to an undefined method
         Symfony\Contracts\Translation\TranslatableInterface::getParameters().
  30     Call to an undefined method
         Symfony\Contracts\Translation\TranslatableInterface::getDomain().
 ------ -----------------------------------------------------------------------
```

Commits
-------

053f62c [Chore] Fix phpstan issues
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants