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

Issues with insert tags in 5.3+ #7183

Draft
wants to merge 1 commit into
base: 5.3
Choose a base branch
from

Conversation

Toflar
Copy link
Member

@Toflar Toflar commented May 3, 2024

We have multiple problems to solve in 5.3+:

  • First of all, I noticed that all Twig templates using |insert_tag will currently use replaceInline(). This means, {{date::whatever}} will never work reliably as it never uses a fragment at the moment.
  • As far as I can see, we never allow inline fragments to affect the cache settings. Meaning you can create an InsertTagResult with getExpiresAt() or cache tags but it will never affect anything unless you specified asFragment: true. We should probably always use the inline renderer if asFragment is set to false (which would require all insert tags to be registered as fragments though, otherwise our FragmentHandler won't care).
  • Currently, the automated query parameters for fragment insert tags will cause cache flooding because on every single page there's both, the pageId and also the request which will cause a cache entry for every single page even though they are - in case of {{date::Y}} - completely irrelevant.

Imho we should not pass on any request parameters. I know this would be kind of a BC break but on the other hand, it's also a bugfix. If you want a fragment insert tag, you should make sure, the insert tag contains all of its information needed to render. So instead of dynamically resolving some format based on the page ID, the format should be part of the insert tag.

But not sure here as currently the implementation by @ausi assumes, insert tag instances are immutable.

This is just a quick hack here to simplify discussion.

Related: #7178

@Toflar Toflar changed the title Just a quick draft to discuss at the dev meeting maybe Issues with insert tags in 5.3+ May 3, 2024
@leofeyer leofeyer added this to the 5.3 milestone May 4, 2024
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