Skip to content

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

Merged
Kocal merged 1 commit intosymfony:2.xfrom
Kocal:doc-twig-components-computed-expose-in-template
Dec 22, 2024
Merged

[TwigComponent] Document about unwanted behavior with ExposeInTemplate and computed methods#2456
Kocal merged 1 commit intosymfony:2.xfrom
Kocal:doc-twig-components-computed-expose-in-template

Conversation

@Kocal
Copy link
Copy Markdown
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 Documentation Improvements or additions to documentation label Dec 19, 2024
@smnandre
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Documentation 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