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: Attempt to obtain mutable reference. #190

Closed
wants to merge 1 commit into from

Conversation

jalcine
Copy link
Contributor

@jalcine jalcine commented Mar 27, 2022

I've come into a situation where I need to call Tera::full_reload so I can reload templates in a development environment (versus respawning this application). I'm not sure if allowing for a From<Arc<Tera>> for TeraHandler would have helped here (because then I can control the instance of Tera being used (to a degree).

I know I opened this up as a PR versus going into discussions—mainly because it seemed simple enough. However, I can't get this imported into my project locally so I wasn't able to fully test this.

@jbr
Copy link
Contributor

jbr commented Mar 27, 2022

I'll take a look at this; I don't think we want to expose Arc::get_mut without a lot of large warnings because it will panic if the application is being run. If we need to expose mutable access to the inner Tera, it will need to be in an RwLock to ensure we don't panic

@jalcine
Copy link
Contributor Author

jalcine commented Mar 27, 2022

Gotcha. And I can imagine something like that would change the internal representation a bit (perhaps to something like RefCell). I'm going to see if I can live without this and keep tinkering.

@jbr
Copy link
Contributor

jbr commented Mar 27, 2022

How are you triggering the reload? Is it a fs-watch driven thing? Do you think it might make sense to make "reload on each request" a development mode option?

@jalcine
Copy link
Contributor Author

jalcine commented Mar 28, 2022 via email

@jalcine
Copy link
Contributor Author

jalcine commented Mar 29, 2022

Yeah, I've been just doing the following in my list of handlers, and it works well for me. I think I can just put this behind a feature flag in a separate function. The performance isn't really measurable yet (I only have about ten pages there):

            Box::new(|conn: Conn| async move {
                templates::TeraHandler::new("web/dist/**/*.html")
                    .run(conn)
                    .await
            }),

I actually do something similar in the code running my personal site, only to allow for runtime swapping of themes. If I do notice anything performance-wise, I'll come back with a better PR for that.

(Originally published at: https://jacky.wtf/2022/3/ZV/ZVqfcJwEH7Mo2g-hSTvCnXSb)

@jalcine jalcine closed this Mar 29, 2022
@jalcine jalcine deleted the tera-allow-mut branch March 29, 2022 18:33
@jbr
Copy link
Contributor

jbr commented Mar 29, 2022

I'm a few days backlogged on trillium, but I do think this is a general purpose utility that people might want, so I'll make an issue to represent this. One of the outstanding design issues that I haven't really addressed is whether there should be some explicit notion of "development mode," and I think there are good arguments on both sides of that issue

@jalcine
Copy link
Contributor Author

jalcine commented Mar 30, 2022

That sounds good to me—in that case, I can probably spend some time this weekend looking into making this PR a bit more reasonable. In the case I had, just reloading the templates on disk would have been enough, so having a private flag that inform the handler to call full_reload every time it's run would work. Shouldn't be a big change, but it'd at least allow for this particular use case to work.

I think doing this removes it from being a development-specific use case (which falls into the case of my personal site) but still allows it to be used in such a context (for this Webmention server app I'm making)

.

(Originally published at: https://jacky.wtf/2022/3/nR/nRJ9TgFL45dF2Swh4UhxXYL7)

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

Successfully merging this pull request may close these issues.

None yet

2 participants