Remove github.com/valyala/fasttemplate dependency

[blkperl]

Summary

We should investigate whether valyala/fasttemplate can be replaced with the standard library text/template or a maintained alternative.

https://pkg.go.dev/text/template

---

Message from the maintainers:

Love this enhancement proposal? Give it a 👍. We prioritise the proposals with the most 👍.

[crenshaw-dev]

Maybe expr too? Been a while since the last release.

[blkperl]

I was limiting my search to dependencies that have not released in the last 2 years. expr has recent commits so perhaps they release yearly.

[djdongjin]

Hi @blkperl ,

I took a look at replacing the mentioned pkg with text/template. IMO the following should be done, given that text/template doesn't support Template.ExecuteFunc directly as in fasttemplate, but only uses a FuncMap which is basically a map from function names to actual functions:

1. Move the executed func to a named function, e.g., execFunc; also change its input to a string (tag) and outputs to a string (parsedString) and error.

2. In NewTemplate, first we should add the function name to every {{tag}} (basically, strings.ReplaceAll(s, "{{", "{{execFunc "), so that the function will be triggered for every tag.

3. Then in NewTemplate, we create a template by the following:

funcMap := template.FuncMap{
    "execFunc": execFunc
}

tmpl, err := template.New("templateTest").Funcs(funcMap).Parse(s)
// ...

4. Then, in the Replace function, we only need this:

replacedTmpl := &bytes.Buffer{}
err = tmpl.Execute(replacedTmpl, struct{}{}) // no data needed.
// ...

[blkperl]

Hi @djdongjin,

Thanks for taking a look at this. Would you like to open a PR with your proposal? If we would could benchmark the change as well then I think that would help with getting it merged.

@alexec @terrytangyuan any thoughts on the implementation detailed above?

[djdongjin]

Hi @blkperl,

Yes, will open a PR. Just want to see if there is any suggestion before implementation. :)

Also is there existing test/benchmarks for the template, or should be included in the PR as well.

Thanks.

[alexec]

There should be a lot of test coverage for templates, so it should be easy to know if this is backwards compatible.

I'm not expecting this to impact performance, but that can be verified by reading stress.md, capturing the profiles and seeing there is anything odd in "allocs" or "cpu" profiles.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[tfurmston]

Is anyone working on this issue?

It looks stale, so I would like to pick it up.

[tfurmston]

OK, so this is what I have so far.

It is probably a bit rough and it still doesn't work for nested templates yet.

However, it is passing on all tests in template_test.go. Also, other than the tests in TestNestedReplaceString, all of the tests in replace_test.go are also passing.

My next step is to work out how to manage nested templates. For that, it seems to me that I am going to have to convert a nested template, e.g., here, to be in the form that text/template expects, e.g., as in this section. Does that seem reasonable?

Also, would be greatly appreciated if someone could look at what I have done so far to see if it is semi-sensible. :)