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

[TwigComponent] Document about unwanted behavior with ExposeInTemplate and computed methods #2456

Merged

Conversation

Kocal
Copy link
Member

@Kocal Kocal commented Dec 19, 2024

Q A
Bug fix? no
New feature? no
Issues Fix #...
License MIT

As spotted when working on my application, the following code triggers two times the methods transactions() because of the presence of #[ExposeInTemplate] (because before I used transactions instead of computed.transactions):

	#[ExposeInTemplate]
    public function transactions(): array
    {
        // SQL query to database
    }
{{ computed.transactions|length }}

When I remove #[ExposeInTemplate], then only one SQL query is made.

@Kocal Kocal added the docs Improvements or additions to documentation label Dec 19, 2024
@smnandre
Copy link
Member

Nothing illogical to me, but a mention in documentation won't hurt and clarify this case. 👍

Could you maybe use a "tip" block instead of a "warning" one here ?

.. warning::

Ensure to not use the ``ExposeInTemplate`` attribute on a computed method,
otherwise the method will be called twice instead of only once, leading to
Copy link
Member

Choose a reason for hiding this comment

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

It kinda leads to present it as a bug... but it is not illogical.

The calls are not made at the same time so the "twice" is maybe a bit missleading
(first time to build the context of the templates, before render, and one by you calling the cache object).

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you think instead, we should fix this behavior?

I mean, throw an exception if the user tries to call a computed method with #[ExposeInTemplate] ?

Copy link
Member

Choose a reason for hiding this comment

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

I am not sure we can. BUT we can maybe populate the computed before the render if method as the attribute. There are some limitations but this could work.

Lets update doc for now and see after if we change and how wdyt ?

(but id rather not use warning or any tone that can stress or frighten users 😇)

@Kocal Kocal force-pushed the doc-twig-components-computed-expose-in-template branch from 647f5f5 to 16cd48a Compare December 20, 2024 07:46
Copy link
Member

@smnandre smnandre left a comment

Choose a reason for hiding this comment

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

We may want to make all these more clear with schema later..

But this precision is usefull right now, thx!

@carsonbot carsonbot added Status: Reviewed Has been reviewed by a maintainer and removed Status: Needs Review Needs to be reviewed labels Dec 21, 2024
@Kocal Kocal merged commit cddafa8 into symfony:2.x Dec 22, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to documentation Status: Reviewed Has been reviewed by a maintainer TwigComponent
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants