feat!: add support for type parameter #272
Conversation
|
I like the idea of having a seperate Something like: type TokenFor[T Claims] struct{ /*… */ }
type Token = TokenFor[Claims]The „new“ |
costela
force-pushed
the
switch-to-generics-for-claims
branch
from
4374bd1
to
b3d339a
Compare
|
@oxisto yes, that's a good point with the type aliases! I'll rebase and see if we can still make this backward compatible. Sounds good? |
Sure! |
costela
force-pushed
the
switch-to-generics-for-claims
branch
from
20b02ba
to
fea6835
Compare
|
Rebase done ✔️ I don't think we can achieve 100% backward-compatibility since The other breaking change is more of an opinion: I personally always found the passing-in of |
costela
force-pushed
the
switch-to-generics-for-claims
branch
from
fea6835
to
c47e5da
Compare
|
@oxisto did you (or any other maintainer) have a chance to look at this? 😇 |
There was a problem hiding this comment.
Initial assessment done. Looks good so far. I still do not quite know what to do with it in our roadmap though. I guess we won't include this in the initial v5 release.
Possibly we can get it to like 99,999% backwards compatibility, I will try to check this in a second round of reviews
| Signature string // Signature is the third segment of the token. Populated when you Parse a token | ||
| Valid bool // Valid specifies if the token is valid. Populated when you Parse/Verify a token | ||
| } | ||
|
|
||
| // Token is an alias for TokenFor[Claims], for backward compatibility. | ||
| type Token = TokenFor[MapClaims] |
There was a problem hiding this comment.
We probably want something like this for even more compatibility:
| type Token = TokenFor[MapClaims] | |
| type Token = TokenFor[Claims] |
|
|
||
| // Token represents a JWT Token. Different fields will be used depending on | ||
| // Keyfunc is an alias for KeyfuncFor[Claims], for backward compatibility. | ||
| type Keyfunc = KeyfuncFor[MapClaims] |
There was a problem hiding this comment.
| type Keyfunc = KeyfuncFor[MapClaims] | |
| type Keyfunc = KeyfuncFor[Claims] |
| @@ -111,8 +117,8 @@ func Parse(tokenString string, keyFunc Keyfunc, options ...ParserOption) (*Token | |||
| // embed a non-pointer version of the claims or b) if you are using a pointer, | |||
| // allocate the proper memory for it before passing in the overall claims, | |||
| // otherwise you might run into a panic. | |||
| func ParseWithClaims(tokenString string, claims Claims, keyFunc Keyfunc, options ...ParserOption) (*Token, error) { | |||
| return NewParser(options...).ParseWithClaims(tokenString, claims, keyFunc) | |||
| func ParseWithClaims[T Claims](tokenString string, keyFunc KeyfuncFor[T], options ...ParserOption) (*TokenFor[T], error) { | |||
There was a problem hiding this comment.
I would probably keep the claims parameter here at least for now, maybe remove it in a future update. The reason is that there are also some cases in which you need to properly initialise an embedded claims struct (in a custom claim), when using a pointer and this is only possible if the claim is provided in the function.
| func NewParser(options ...ParserOption) *Parser { | ||
| p := &Parser{ | ||
| validator: &validator{}, | ||
| func NewParser(options ...ParserOption) *Parser[MapClaims] { |
There was a problem hiding this comment.
| func NewParser(options ...ParserOption) *Parser[MapClaims] { | |
| func NewParser(options ...ParserOption) *Parser[Claims] { |
|
This is a nice feature, it would be great if it could be merged |
|
Any updates on when this will be merged? |
|
@hantonelli sorry, this totally got lost in the day-to-day work. I'll try to rebase and address the points above 🤞 |
Outside of generics, does this PR have a feature that's missing from the current /v5 library? |
This PR introduces the type parameters to the API, allowing token claims to be accessed directly using their concrete types, without type assertions.
The change bumps the minimal go version to 1.18.
It also:
claimsparameter from all methods; replaced by the typejwt.NewParserForgeneric initializer (keeping thejwt.NewParseras ajwt.MapClaimsvariant)Parser.ParseWithClaims(claims can be derived from the parser)request.ParseRequestWithClaims(requiring explicit typing)v5branch, but should probably be rebased on a newv6branch if v5 gets released without it.Benchmarks