-
Notifications
You must be signed in to change notification settings - Fork 5
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
Add http-core package #430
Add http-core package #430
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks nice! I really like the abstraction. I added some minor details to polish.
packages/http/libraries/core/src/adapter/InversifyHttpAdapter.ts
Outdated
Show resolved
Hide resolved
packages/http/libraries/core/src/adapter/InversifyHttpAdapter.ts
Outdated
Show resolved
Hide resolved
case RequestMethodParameterType.NEXT: { | ||
return next; | ||
} | ||
case RequestMethodParameterType.PRINCIPAL: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious about this :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This functionality returns the user from the request in the current library. We currently do not have an authentication service, which is why it is disabled.
Anyways, if we enable authentication services, do we want to keep this functionality with the same name and usage? I'm not sure.
… into feat/add-http-support
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks great, let's fix CI issues and I think we can merge
Added