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

🤗 [Question]: V3: ctx.Render not longer accepts types other than fibre.Map #3219

Open
3 tasks done
m13t opened this issue Nov 28, 2024 · 4 comments
Open
3 tasks done

Comments

@m13t
Copy link

m13t commented Nov 28, 2024

Question Description

I'm curious why the API for the fiber context no longer accepts the any type for render binding parameters and only accepts fiber.Map?

The abstracted template engines still support bind parameters allowing the passing of structs (or indeed a map) so I'm wondering if there are plans to change the template engine implementation to match too?

I feel like migrating from V2 to V3 with this change will result in a lot of unnecessary refactoring for users where structs that are used throughout an application lifecycle as well as being passed to templates, now need to be converted to a map structure.

Code Snippet (optional)

No response

Checklist:

  • I agree to follow Fiber's Code of Conduct.
  • I have checked for existing issues that describe my questions prior to opening this one.
  • I understand that improperly formatted questions may be closed without explanation.
Copy link

welcome bot commented Nov 28, 2024

Thanks for opening your first issue here! 🎉 Be sure to follow the issue template! If you need help or want to chat with us, join us on Discord https://gofiber.io/discord

@efectn
Copy link
Member

efectn commented Dec 4, 2024

In Fiber v2 it accepts any but it asserts the bind parameter into Map, so if the argument's actual type is not fiber.Map then it's not used at all. In v3, we made it Map and removed assertion step. Therefore, it should not really be changing any functionality or behavior. Can you explain whether it changes anything @m13t ?

@m13t
Copy link
Author

m13t commented Dec 4, 2024

Thanks for the response @efectn.

So the reason I raise this is that all the template engines accept structs or maps as bind parameters to be used in templates, so this feels like a big change to force consumers of Fiber v3 to have to convert structs to maps for passing to templates.

I can see in the code for V2 that it only sets bind to a map if there bind is nil and it must be a map in order to pass locals from the request context to the template. However, passing structs to the template engine works as expected in V2, structured properties are able to be used in templates.

So just to clarify, this is quite a significant change to template functionality in V3, hence the question why the decision was made to change this?

See the following:

fiber/ctx.go

Lines 1583 to 1585 in 6968d51

if bind == nil {
bind = make(Map)
}

fiber/ctx.go

Line 1603 in 6968d51

if err := app.config.Views.Render(buf, name, bind, layouts...); err != nil {

@efectn
Copy link
Member

efectn commented Dec 4, 2024

Thanks for the response @efectn.

So the reason I raise this is that all the template engines accept structs or maps as bind parameters to be used in templates, so this feels like a big change to force consumers of Fiber v3 to have to convert structs to maps for passing to templates.

I can see in the code for V2 that it only sets bind to a map if there bind is nil and it must be a map in order to pass locals from the request context to the template. However, passing structs to the template engine works as expected in V2, structured properties are able to be used in templates.

So just to clarify, this is quite a significant change to template functionality in V3, hence the question why the decision was made to change this?

See the following:

fiber/ctx.go

Lines 1583 to 1585 in 6968d51

if bind == nil {
bind = make(Map)
}

fiber/ctx.go

Line 1603 in 6968d51

if err := app.config.Views.Render(buf, name, bind, layouts...); err != nil {

Thanks for the clarification; now, i get you. I don't really remember the historical reason behind this change despite the fact that the change was commited by me :D I think you are right, this needs to be reverted as most of the template engines also support. What do you think @ReneWerner87 @gaby

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants