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

🔥 Consider strong typing for API #178

Closed
alex-shpak opened this issue Feb 22, 2020 · 7 comments
Closed

🔥 Consider strong typing for API #178

alex-shpak opened this issue Feb 22, 2020 · 7 comments

Comments

@alex-shpak
Copy link

alex-shpak commented Feb 22, 2020

Is your feature request related to a problem?
Hi!

In Go (IMO) it is considered bad practice to use interface{} if strongly typed API can be implemented.
I understand that idea is to have similarity with the expressjs library, but now developer has to visit documentation/source to identify what are proper params types for the routing function.

Also I think strongly typed API is one of things developers look after when choosing framework. It also can decrease amount of runtime checks and will show errors/mistypes during compile time.

Describe the solution you'd like
As an example for routing function:

func (app *App) Get(
    path string, 
    handler func()(ctx *Ctx), 
    middlewares ...func()(ctx *Ctx)
) App* {
    ...
}

Describe alternatives you've considered
Leave it as it is ¯\(ツ)

@welcome
Copy link

welcome bot commented Feb 22, 2020

Thanks for opening your first issue here! 🎉 Be sure to follow the issue template!

@Fenny
Copy link
Member

Fenny commented Feb 22, 2020

Fiber started as a personal project to be able to rewrite all my node apps into Go. I tried to stick to the same workflow as Express by using interfaces when a function had multiple purposes, now I know that this is not the best way to go.

I thought about going with a 100% strong typed API weeks ago, but Fiber got a lot of users within a short period of time. So changing Fiber to be a fully strong typed framework will break the current API, but we can take notes for v1.8

@alex-shpak
Copy link
Author

I see.
Well, it is up to library maintainers to decide. I feel like having non strongly typed API can turn off some Go developers. Would be nice to have ¯\_(ツ)_/¯.

In my case I added fiber to dependencies, started to write main function to try around and hit app. autocompletion, saw args ...interface{} and was like: damn... now I need to go find docs.

And not to mention I'm a member of strongly typed camp. 😄

@hyingreborn
Copy link

hyingreborn commented Feb 23, 2020

agree,Fiber made it easy for me to migrate from nodejs to Go。
The reason I chose to learn Go instead of TypeScript:

  • A strong typed language can reduce errors and catch exceptions more easily
  • Moderate development efficiency and adequate operational performance
  • Cross-compile into binaries for easy distribution and deployment

Now, it's true that docs are often needed ,and Some exceptions can only be caught at runtime.

@Fenny
Copy link
Member

Fenny commented Feb 23, 2020

I have discussed this with @koddr, and we want to share our conclusion on this matter.

Most methods that use the ...interface{} argument are used for app.METHODS to register routes with an optional path. We think that this is straightforward because this is the first thing you learn visiting Fiber or coming from Express.

Now there are some Context methods that accept ...interface{} as an argument:
ctx.Body, ctx.Cookies, ctx.Format

When you managed to understand the Route handlers, you are left with these 3 +/- methods where you need docs. So I think Fiber is still a strong typed API besides these 3 methods. But we could improve the above methods and maybe all Route handlers (except Use).

I started working on clarifying some *Ctx functions, these are safe changes to make.
master...Fenny:master

We can change all app.METHODS to have a required (path string, handlers ...func(*Ctx)) for the following methods:
Static, Connect, Put, Delete, Head, Patch, Options, Trace, Get, All.

This is a big change, and I might create an Poll for this and take a vote.

@alex-shpak
Copy link
Author

I like that 👍
I guess Static method would be Static(string, ...string). Also for method functions something like this might be good: Get(path string, handler func(*Ctx), handlers ...func(*Ctx) so it requires to have at least one handler, or more.

I think having strong typing could cleanup router.go@register function and make it more safe.

I cant help with some PRs if you have something specific in mind.

This was referenced Feb 25, 2020
@Fenny
Copy link
Member

Fenny commented Feb 27, 2020

Thanks @alex-shpak for your input, all suggestions will be addressed in v1.8.0.

PS: We went with Get(path string, handlers ...func(*Ctx)) because we don't want to confuse people with the order of where to place the middleware handlers.

app.Get("/", secretpage(), auth()) 
// Might cause unintended results since auth() will run after the secretpage

app.Get("/", auth(), secretpage())
// We think with the current arg types this will be encouraged.

I'm closing this issue because v1.8.0 will be released soon, feel free to re-open for more ideas/suggestions!

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

No branches or pull requests

3 participants