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

Render method should take tags #672

Closed
raymond-lam opened this issue Aug 16, 2018 · 2 comments
Closed

Render method should take tags #672

raymond-lam opened this issue Aug 16, 2018 · 2 comments

Comments

@raymond-lam
Copy link
Contributor

The parse takes a tags argument, which overrides mustache.tags. There is no way to pass in a tags argument into the render method, however. In addition to the unfortunate asymmetry that this causes, this has caused some amount of confusion/concern with regard to the caching behavior of parse. See issue #669 for more discussion.

@raymond-lam
Copy link
Contributor Author

raymond-lam commented Aug 17, 2018

The more I think about this, the more I am in favor of the changes proposed here and in issue #669. The README.md actually suggests using parse as a means of overriding Mustache.tags for rendering a particular template, but this violates the Single Responsibility Principle; parse should take a template and some arguments (namely tags) and, well... parse... and nothing more. (We of course cache for performance, but the cache should be correct and behave as expected.)

Adding tags to render to "compensate" for "correcting" the caching behavior brings us closer to Least Astonishment: render will render according to the template, other arguments it is given, and Mustache.tags if it is not overridden by such arguments. The arguments and Mustache.tags are totally transparent to the caller. More Astonishing than this is render behaving depending on the template, arguments, and maybe Mustache.tags unless an opaque/hidden value was set by calling a different method whose original and main purpose was not actually to override Mustache.tags for all subsequent render calls.

Thanks to @mbrodala and @ilkovich for suggesting this.

@phillipj
Copy link
Collaborator

Couldn't agree more with your rationale above. Glad you've found an improvement that doesn't require the using developers to grasp the interconnection between .render() | .parse() | .tags 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants