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

Make twig/extra-bundle optional #6170

Merged

Conversation

wbloszyk
Copy link
Member

@wbloszyk wbloszyk commented Jun 26, 2020

Subject

sonata-project/intl-bundle and twig/intl-extra register format_datetime what make them in conflict.
twig/extra-bundle require-dev twig/intl-extra and auto-register it in twig. This will not allow use last SonataAdminBundle releases with some bundles like ecommerce.

I am targeting this branch, because this change respect BC.

Closes #6169.

Changelog

### Changed
- Move `twig/extra-bundle` from `require` to `suggest`

### Fixed
- Fix error with missing `u` filter when `twig/extra-bundle` is not registred

@wbloszyk wbloszyk force-pushed the fix_string_extension_load branch 2 times, most recently from 7ae1d78 to ab77c6a Compare June 26, 2020 14:15
@SonataCI
Copy link
Collaborator

Could you please rebase your PR and fix merge conflicts?

@wbloszyk wbloszyk force-pushed the fix_string_extension_load branch from ab77c6a to 781e826 Compare June 29, 2020 15:21
@SonataCI
Copy link
Collaborator

Could you please rebase your PR and fix merge conflicts?

@wbloszyk wbloszyk force-pushed the fix_string_extension_load branch 3 times, most recently from e56741f to c07db02 Compare June 30, 2020 08:27
@wbloszyk wbloszyk marked this pull request as ready for review June 30, 2020 09:11
core23
core23 previously approved these changes Jun 30, 2020
@wbloszyk
Copy link
Member Author

wbloszyk commented Jun 30, 2020

I found another bug here :(
All working correct but minimum one of bundles with this change must be register before TwigBundle. Otherwise compiler pass from twig will not register StringExtension as twig.extension.

Update

Add priority resolve this issue:

$container->addCompilerPass(new TwigStringExtensionCompilerPass(), PassConfig::TYPE_BEFORE_OPTIMIZATION, 1);

@wbloszyk wbloszyk force-pushed the fix_string_extension_load branch from c07db02 to b92b12e Compare June 30, 2020 10:28
@wbloszyk wbloszyk requested a review from core23 June 30, 2020 10:28
@wbloszyk wbloszyk force-pushed the fix_string_extension_load branch from b92b12e to 7d4f19c Compare June 30, 2020 10:39
@wbloszyk wbloszyk force-pushed the fix_string_extension_load branch from 7d4f19c to dbbe2ff Compare June 30, 2020 11:38
@jordisala1991 jordisala1991 requested a review from a team June 30, 2020 12:48
@phansys
Copy link
Member

phansys commented Jun 30, 2020

Note: PHPStan is failing.

@wbloszyk
Copy link
Member Author

PHPStan failing becouse CoreBundle id optional. I think this should be fix in dev-kit.

@jordisala1991
Copy link
Member

No, it is fixed here: #6161

@VincentLanglet
Copy link
Member

Let's merge this then ; the phpstan will be fixed.

@jordisala1991 jordisala1991 merged commit 36902d9 into sonata-project:3.x Jun 30, 2020
@jordisala1991
Copy link
Member

Thank you @wbloszyk

@wbloszyk wbloszyk deleted the fix_string_extension_load branch June 30, 2020 18:31
@greg0ire greg0ire added the patch label Jun 30, 2020
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.

Unknown "u" filter
7 participants