Skip to content

Mustachio: avoid calling Iterable getter multiple times #2546

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

Merged
merged 3 commits into from
Mar 1, 2021

Conversation

srawlins
Copy link
Member

@srawlins srawlins commented Feb 25, 2021

Take this template, where widgets is an Iterable property:

Text {{#widgets}} {{name}} {{/widgets}}.

A mustache renderer should assume that the widgets getter may have side
effects and may be expensive to compute, and it should only be called one
time. Currently, Mustachio will call this getter 2 or 3 times, as the behavior
differs if the section is inverted, and if the Iterable is empty.

This change is actually quite readable, and simplifies the code. The magic
sauce is in the lazy Iterable returned by renderIterable. If Mustachio does
not wish to render the elements, then the lazy map will not be evaluated.

Take this template, where `widgets` is an Iterable property:

Text {{#widgets}} {{name}} {{/widgets}}.

A mustache renderer should assume that the `widgets` getter may have side
effects, and it should only be called one time. Currently, Mustachio will call
this getter 2 or 3 times, as the behavior differs if the section is inverted,
and if the Iterable is empty.

This change is actually quite readable, and simplifies the code. The magic
sauce is in the lazy Iterable returned by `renderIterable`. If Mustachio does
not wish to render the elements, then the lazy map will not be evaluated.
@google-cla google-cla bot added the cla: yes Google CLA check succeeded. label Feb 25, 2021
@srawlins srawlins requested a review from jcollins-g February 25, 2021 22:56
@coveralls
Copy link

coveralls commented Feb 25, 2021

Coverage Status

Coverage increased (+0.002%) to 91.714% when pulling ffb22c6 on srawlins:mustachio-q into 76206db on dart-lang:master.

@parlough
Copy link
Member

@srawlins Can you merge master into your PRs to get the tests passing?

Copy link
Contributor

@jcollins-g jcollins-g left a comment

Choose a reason for hiding this comment

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

LGTM

@srawlins srawlins merged commit f8d62e0 into dart-lang:master Mar 1, 2021
@srawlins srawlins deleted the mustachio-q branch March 1, 2021 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Google CLA check succeeded.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants