8000 [TwigBundle] Add "use_yield" option to allow opting in for this new rendering mode and skip a deprecation by nicolas-grekas · Pull Request #53805 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[TwigBundle] Add "use_yield" option to allow opting in for this new rendering mode and skip a deprecation #53805

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

Closed
wants to merge 1 commit into from

Conversation

nicolas-grekas
Copy link
Member
@nicolas-grekas nicolas-grekas commented Feb 6, 2024
Q A
Branch? 5.4
Bug fix? yes
New feature? no
Deprecations? no
Issues -
License MIT

Adapting to upcoming changes in twig 3.9.
This will need a corresponding recipe update so that new projects don't start with a deprecation.

echo $data;
}
} else {
$this->template->displayBlock($blockName, $context, $this->resources[$cacheKey]);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it looks like we're using an internal API here 🙈

// By contract,This method can only be called after getting the resource
// (which is passed to the method). Getting a resource for the first time
// (with an empty cache) is guaranteed to invoke loadResourcesFromTheme(),
// where the property $template is initialized.

if ($this->template instanceof YieldingTemplate) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we actually still need to use the internal class Template in $this->template, forcing us to use more internal classes there ?
Couldn't we instead rely on the public API of TemplateWrapper, which has a renderBlock method (and btw, meging globals would also be useless as Twig would be doing it in its public entry points)

weaverryan added a commit to symfony/ux that referenced this pull request Feb 9, 2024
…deprecation notice in CI (evertharmeling)

This PR was merged into the 2.x branch.

Discussion
----------

[StimulusBundle/TwigComponent] Fix "use_yield" to "true" deprecation notice in CI

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| Issues        |
| License       | MIT

Only this one remains (in StimulusBundle), but I can't find a direct implementation related to the `Environment` in that test. Maybe also related to symfony/symfony#53805 or thoughts anyone?

```
1x: Since twig/twig 3.9.0: Not setting "use_yield" to "true" is deprecated.
    1x in StimulusControllerLoaderFunctionalTest::testFullApplicationLoad from Symfony\UX\StimulusBundle\Tests\AssetMapper
```

Commits
-------

1dbb7e0 [StimulusBundle] Set "use_yield" option to fix deprecation message
symfony-splitter pushed a commit to symfony/ux-twig-component that referenced this pull request Feb 9, 2024
…deprecation notice in CI (evertharmeling)

This PR was merged into the 2.x branch.

Discussion
----------

[StimulusBundle/TwigComponent] Fix "use_yield" to "true" deprecation notice in CI

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| Issues        |
| License       | MIT

Only this one remains (in StimulusBundle), but I can't find a direct implementation related to the `Environment` in that test. Maybe also related to symfony/symfony#53805 or thoughts anyone?

```
1x: Since twig/twig 3.9.0: Not setting "use_yield" to "true" is deprecated.
    1x in StimulusControllerLoaderFunctionalTest::testFullApplicationLoad from Symfony\UX\StimulusBundle\Tests\AssetMapper
```

Commits
-------

1dbb7e0f [StimulusBundle] Set "use_yield" option to fix deprecation message
@nicolas-grekas
Copy link
Member Author

Closing in favor of #53913

@nicolas-grekas nicolas-grekas deleted the twig-use_yield branch February 14, 2024 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0