pilcrowonpaper / arctic

OAuth 2.0 clients for popular providers
MIT License
1.11k stars 65 forks source link

Feat: Make decodeIdToken generic #172

Closed BryanAbate closed 1 month ago

BryanAbate commented 2 months ago

Often times we know the form of the payload and it would improve DX if we could type the resulting object returned by decodeIdToken. I have a branch with the code ready and will make a pull request if this proposal is accepted. See here: https://github.com/pilcrowOnPaper/arctic/compare/v2...BryanAbate:arctic:v2-typed-decode?expand=1

pilcrowonpaper commented 2 months ago

Can't you just use type assertions? It's much clearer that you're not validating the value type too.

const payload = decodeIdToken(idToken) as TokenPayload
BryanAbate commented 2 months ago

Functionally, yes it's the same. However, imho, it's cleaner to use a generic. Indeed, a generic parameters is equivalent to saying "The payload is of type T" where when using a type assertion, it's equivalent to saying "The return type of the function is of type T" which not only leaks implementation details, but also remove some type safety. One example scenario where type safety is a concern: imagine the function in the futur returns object | null the type assertion would not throw any error (you can see this issue in action here: https://www.typescriptlang.org/play/?#code/KYDwDg9gTgLgBAMwK4DsDGMCWEVwCbBoQECSeAKhANbAoAUmF1tAXHAM4xSYoDmAlGwgAjAFaF4AHzgokAGzlwA3gCgAkFwCey9WqjAYSKLiUBfANzrTcNAEMYaABZw6wfjrUbHUCAHcZwP4AolA+UHQARCQoAG62coxwJAAicDDMKBH8lmqmKnkqRCicacAlALz4hMTAZJQ09ADkMGUwje627MotnGyc3Hx5QA).

But I agree that the cast make it clear that there is no validation happening, although this is true for all TS generics. If you still prefer to keep it without generic, no problem, you can close this issue.