-
Notifications
You must be signed in to change notification settings - Fork 71
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
Node API feedbacks and potential improvements. #526
Comments
Hi @cyrilchapon thanks for the extensive feedback.
That being said, the user stories you pointed out are all valid, but rare. We are open to making this change if we see these user stories enough, by real projects.
The supertokens.init on the backend has very little to do with the supertokens core. When you do supertokens.init, you inform the sdk about configs that it needs to use when recipe functions are called or verifySession middleware is called.
You are not declaring the session recipe in the core instance. You are telling the SDK that this recipe is to be enabled for use with these set of configurations.
This is a fair point that adding the middleware like
GetSession does not do res.send. It does throw an error which is then handled by the supertokens errorHandler (that you added to your app), which in turn does res.send. If you like, you can catch the error from getSession and it handle it however you like. Finally, the idea you suggested at the end looks really good. We shall think about this, but in our experience, in reality, it usually ends up making the dev ex more complex with all the different parts of what the sdk does. Please feel free to further argue with my comments :) |
Hi,
New to Supertokens, I'm implementing a backend API with Auth, and was sort of surprised of the code architectural / API choices for supertokens-node. I'll try to make it clear and objective, but this is inherently opiniated and discussion-centric.
To explain myself, I'll come from this homemade example featuring email/pwd and session management :
This design appear strange to me for several reasons. Here in no particular order :
Why exposing that global import variable, in the first place ?
The first one :
init()
stMiddleware()
under the hoodThe second one :
Why do I need to tell the used framework on the core instance ?
(probably linked to first concern)
The fact we pass the backend framework seems to be a witness something should not be done right inside the core
Supertokens
.Why responses customization is on the core instance ?
Supertokens
instance has nothing to do with express in the first place;verifySession
has, which should hold the responses management logic.Why Session recipe declaration on the Supertokens core instance in the first place ?
Strange magic route attachment
When reading this :
It's barely clear the auth routes will be attached to
/auth
. Now separateSupertokens.init()
in a different file, it leads to :GetSession magic
res
behaviorCalling
Magically calls
res.send
; while we basically expect a thrown error.Some ideas :
@supertokens-node/core
@supertokens-node/express
@supertokens-node/koa
With clear separation of concerns : the core would be a supertokens rest API client; the express would be the wrapper with different levels of abstractions.
The text was updated successfully, but these errors were encountered: