🔥 Feature: Add ctx.Bind for set default var map for template engine

[aztecrabbit]

Please provide enough information so that others can review your pull request:

#1536

Hi, i'm changing binding var type from interface{} to Map, is that ok?

[welcome]

Thanks for opening this pull request! 🎉 Please check out our contributing guidelines. If you need help or want to chat with us, join us on Discord https://gofiber.io/discord

[liaohongxing]

can render be changed to:

Render(*Ctx, io.Writer, string, Map, ...string) error

[ReneWerner87]

can render be changed to:

Render(*Ctx, io.Writer, string, Map, ...string) error

why ?

[ReneWerner87]

@liaohongxing why ?

[aztecrabbit]

@ReneWerner87 Because we can easily implement for this feature (set default var map for tempate engine), and also no need to check binding arg type in every Render func in gofiber/template repo.

https://github.com/gofiber/template/blob/359ca25289df6bd214320ff864103d18051e5b01/jet/jet.go#L206-L252

[ReneWerner87]

the same topic I had just answered here
#1622 (comment)
think it is unnecessary, because currently everything works and simply the bindings can be extended with the values that are needed

always having everything and introducing the context here is not really in line with clean code concepts and also makes writing unittests harder and using them outside the fiber context

[aztecrabbit]

think it is unnecessary, because currently everything works and simply the bindings can be extended with the values that are needed

I'm using fiber not for api, pure using template engine. Sending session data to template engine in every route is too much work for me.

I'll close this pull request. But the feature like in this pull request will be added to fiber or not (maybe in fiber v3)? Because i think feature like this is very important for project that only using template engine.

[ReneWerner87]

do not misunderstand, the "why" referred to the extension/adjustment of the render function with the context (first parameter)

the bind method is already useful and good, the only thing that prevents me from merging it is the adjustment of the type in the declaration, don't know if that causes a breaking change and then users have to make adjustments, even though it wouldn't really be necessary

[aztecrabbit]

I missunderstand. I'm not seeing the source code of this repo for long time. I'm very sorry for this. Maybe i drinking too much coffee today, hahaha. Again, i'm so sorry @ReneWerner87

[ReneWerner87]

is okay, does not matter :D , am always glad about every pull request

[ReneWerner87]

@aztecrabbit can you check my last comments to the pull request, maybe we can make this backward compatible

[liaohongxing]

I hope to change Render(io.Writer, string, interface(), ...string) error to Render(*Ctx, io.Writer, string, interface(), ...string) error in the next major version

Add a Ctx parameter, interface() can be changed to Map, it doesn’t matter

This is very helpful for me to write a custom template engine project, mainly to help me inject session, ctx.Local into the template

@ReneWerner87

[aztecrabbit]

@ReneWerner87 The interface{} to Map think? I think not possible, because interface{} is too abstract for this. I think add this feature in fiber v3 is the best option right now

[ReneWerner87]

fixed with #1754