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

Add a note on tag deprecation example #3594

Closed
wants to merge 1 commit into from
Closed

Add a note on tag deprecation example #3594

wants to merge 1 commit into from

Conversation

michaelKaefer
Copy link

@michaelKaefer michaelKaefer commented Dec 1, 2021

For further info see #2696

.. note::

If you expect your client code to never call your block, but to override your
block, then you should not follow the above example of deprecating a block.
Copy link
Contributor

Choose a reason for hiding this comment

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

That's not really accurate. If you override the block, indeed nothing in the block will be called. But if you call parent, then you will get it. Let's close as it adds more confusion IMHO.

Copy link
Author

Choose a reason for hiding this comment

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

True, what would be really accurate is: If you expect your client code to override your block without ever calling your block then you should not follow the above example of deprecating a block. - I still think there should be a note to signal that it is not possible to deprecate a Twig block (it's only possible if you are lucky and your client code calls your block when overriding it).

Using this tag for deprecating a block makes no sense to me (it still makes sense for deprecating templates though):

  1. If you use {% deprecated '...' %} your users who do not override the deprecated block at all (which is legitimate of course) will face a deprecation - but they should not.
  2. Even worse: if client code overrides the deprecated block and doesn't call the overridden block (which is legitimate of course) the developer gets no deprecation and the app will break on the next major version of your package when you remove the deprecated block. - Here is an example of the confusion that this tag can cause: Update content.html.twig EasyCorp/EasyAdminBundle#4941

It would be nice to note that somehow to warn everybody who wants to use the deprecated tag for deprecating blocks (or the example of deprecating a block should be removed).

Copy link
Contributor

Choose a reason for hiding this comment

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

You're right. I've just opened #3672 to remove the block example (I've replaced it with an example for macros) and added a note to warn about using deprecated for blocks.

@fabpot fabpot closed this Mar 25, 2022
fabpot added a commit that referenced this pull request Mar 26, 2022
This PR was submitted for the 3.x branch but it was merged into the 2.x branch instead.

Discussion
----------

Fix docs about the deprecated tag

Closes #3594

Commits
-------

17142af Fix docs about the deprecated tag
@michaelKaefer michaelKaefer deleted the patch-2 branch March 26, 2022 18:38
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.

None yet

2 participants