Conversation
|
Just noticed an open issue #9 and pretty sure that this PR should partially solve it. |
|
@raoufr I tried to integrate the api utils in the user profile service, but it seems to be complicating things. The way ApiUtils is designed is to have its instances created for each service. const userProfileApi = createApi(baseUrl);For the consumers, to be able to pass the baseUrl to the So for now, we can leave the service as is. Whats remaining in this PR is to add the return types on multiple methods. I think @bittu1028 is going to take care of that. |
| return null; | ||
| } | ||
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
| import type { IUser } from './interfaces/user.interface'; |
There was a problem hiding this comment.
@ajaybushel we need to talk about our convention here make sure we are following the same on all repos including the mobile app. I will setup a meeting to sync up on that
I don't love having a different patternfor this. I am ok with it now to move us forward, as this is relatively abstracted, but teh whole point of apiUtils was to have one common place to make any types of api calls, INCLUDING handling exception, cleaningup, etc. I understand your challenge of adding things like tokens into the headers ... but we shoudl be updating apiUtils.ts to support this, as opposed to doing this one off If this is pressing, i am ok moving this in first as, like i said, it is abstracted enough. @ajaybushel and @bittu1028 , if we have cycles, i'd like for us to ensure we get api utils to support our common microservice calls, instead of having all these different patterns. |
I second @JLeeC's concern. I think we should hold off on this PR and discuss the possibility of enhancing the ApiUtils service, if really needed, then come back here and assess the proposed changes in this PR at that point. If there is a real urgency to get this PR merged in this state, please let us know and let's discuss. |
No description provided.