unrolled / render

Go package for easily rendering JSON, XML, binary data, and HTML templates responses.
MIT License
1.94k stars 146 forks source link

Question: is it possible to change the API of the Render struct to allow setters? #100

Closed mariusor closed 2 years ago

mariusor commented 2 years ago

Hello, I'm using the render package in a web application and lately when doing some load testing there are a lot of panics (mostly related to parsing incomplete templates and trying to close already closed files).

I believe this is due to the fact that the Render struct only accepts configuration options at initialization and when I have request scoped functions that I want to pass to it, I am forced to make the Render initialization request scoped, instead of globally scoped. (ie, every request goroutine will call the render.New() function)

This makes it that all the template parsing is being executed for every request, and when the concurrency is high it results in the panics.

My question(s) are:

Is there a way that I'm missing to pass/modify the function map in a request scope instead of at initialization?

Is there another solution to this problem?

If not, is a pull request where the function map can be set after initialization be agreeable/accepted?


I will try to come up with a minimal repro case, but until I do, I'd like to know if it's worth investing time into this.

Thank you.

unrolled commented 2 years ago

Is this for HTML rendering? If so, you can pass in an HTMLOptions struct which allows you to define template functions: https://github.com/unrolled/render/blob/v1/render.go#L116

mariusor commented 2 years ago

Yes, that looks like something I could use, thank you. :)

However I'm not sure I understand how this API works when the templates contain references to functions that haven't been passed to the initializer function.

When the initializer is compiling the templates, the underlying template.Must panics. I guess this would be the same behaviour for my suggestion in the end, so I must think of an alternative way of delaying the parsing until render time.

mariusor commented 2 years ago

In the end I forked the repo and stripped it down to only the functionality I needed.

I doubt it can ever be converted to an acceptable PR as I removed a lot of stuff. Apologies for wasting your time.