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

Implement YieldingTemplate::display*Block() #3992

Closed
wants to merge 1 commit into from

Conversation

nicolas-grekas
Copy link
Contributor

This should make the migration a bit easier (Symfony uses displayBlock() directly in the Form component.)

@GromNaN
Copy link
Contributor

GromNaN commented Feb 9, 2024

How that would make the migration easier?

The call to displayBlock in Symfony Form is surrounded by ob_start/ob_get_clean. We can fix the calling code in Symfony 5.4+ to check if Twig is using the yielding mode (method_exists($template, 'yieldBlock')) and call it instead.

@nicolas-grekas
Copy link
Contributor Author

This improves the migration by not breaking ppl that will update twig but not symfony. This will happen during the transition period.

@GromNaN
Copy link
Contributor

GromNaN commented Feb 9, 2024

If there is a conflict rule in twig for symfony/form < 5.4.36 and the fix is in the new version bugfix, that would prevent issues.

If this PR is merged, it deserves a deprecation warning as this methods are removed from 4.0.

@nicolas-grekas
Copy link
Contributor Author

nicolas-grekas commented Feb 9, 2024

twig shouldn't know about symfony/form - that's be entering the package coupling arena
the deprecation is possible but not worth it IMHO

@nicolas-grekas
Copy link
Contributor Author

Closing in favor of #3999

@nicolas-grekas nicolas-grekas deleted the display branch February 13, 2024 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants