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

Tera::render_str() takes a &mut self while it can work with a non-mutable reference. #943

Open
k1rowashere opened this issue Oct 31, 2024 · 3 comments

Comments

@k1rowashere
Copy link

Context: I want to render a one off template, while using my custom filters, and macros.

TEMPLATES.render_str() is perfect for that, but it requires a &mut self, which is a problem because I have my Tera as a global static, and I don't want to wrap it in Arc:<Mutex<...>>.

It is mutable because of a call toself.build_inheritance_chains().

May I suggest :

  1. A non-mutable version where it disallows inheritance (using an assert! or return an error).
  2. Replace the existing method, but do the check without rebuilding (requires major refactoring/rewriting some logic to avoid code duplication).

p.s. I am willing to open a PR, but I am not familiar enough with codebase ( yet :) ).

Thank you

@k1rowashere k1rowashere changed the title Tera::render_str() takes a mutable reference to self while it can (potentially) work with non-mutable one. Tera::render_str() takes a &mut self while it can work with a non-mutable reference. Oct 31, 2024
@azemlya
Copy link

azemlya commented Nov 1, 2024

Yes, I don't understand either. If all the objects in Tera are wrapped in Arc, then Mutex is also suggested inside for internal mutability. Otherwise, why Arc? If you wanted to block the ability to change, then it would be better to use Rc for speed. Now, apparently, it is very time-consuming to make such changes...

@Keats
Copy link
Owner

Keats commented Nov 1, 2024

It's not going to change for v1 but we can add option 1 and/or 2 for v2. The initial reasoning was that some people would want inheritance for string rendering but maybe it was an incomplete assumption?

@k1rowashere
Copy link
Author

It's not going to change for v1 but we can add option 1 and/or 2 for v2. The initial reasoning was that some people would want inheritance for string rendering but maybe it was an incomplete assumption?

I think we could still have inheritance without mutating the original templates (maybe in v1?).

We can assume that the chains are already built, just calculate it for the oneoff template?

This would only require changing the implementation of render_str, and would be backwards compatable. (Cargo might give warnings for unnecesary mut refs)

Also i think this would be woth the hassle, because the performance cost for arc mutex would be a lot. (In my use case a lt least,).

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

3 participants