refactor: refactored api routes to be centeral #834
Conversation
mmoehabb
left a comment
mmoehabb
left a comment
There was a problem hiding this comment.
You have really done a great job thinking of and designing this solution. Kudos to you 😄
| | "notifications" | ||
| | "topics"; | ||
|
|
||
| export const ApiRouteBaseRoute = "/api/v1"; |
There was a problem hiding this comment.
The naming here is quiet repetitive. We may call it ApiRouteMainBase.
| apiRoutes.generateUrl({ | ||
| route: { | ||
| base: "auth", | ||
| }, | ||
| type: "base", | ||
| }), | ||
| routes.auth | ||
| ); | ||
|
|
||
| app.use( | ||
| apiRoutes.generateUrl({ | ||
| route: { | ||
| base: "contactRequest", | ||
| }, | ||
| type: "base", | ||
| }), | ||
| routes.contactRequest | ||
| ); | ||
|
|
||
| app.use( | ||
| apiRoutes.generateUrl({ | ||
| route: { | ||
| base: "user", | ||
| }, | ||
| type: "base", | ||
| }), | ||
| routes.user(context) | ||
| ); |
There was a problem hiding this comment.
There is a pattern here:
For any string x:
app.use(
apiRoutes.generateUrl(
route: {
base: x,
},
type: "base"
)
)So it can be re-written as follow:
For any string x:
app.use(getBaseOf(x), ...)| import { ApiRoutes } from "@litespace/utils/routes"; | ||
|
|
||
| const router = Router(); | ||
| const sampleRouts = ApiRoutes.sample.routes; |
There was a problem hiding this comment.
Isn't it ApiRoutes.asset.routes?
| const availabilitySlotRoutes = ApiRoutes.availabilitySlot.routes; | ||
|
|
||
| router.get(availabilitySlotRoutes.find, availabilitySlot.find); | ||
| router.post(availabilitySlotRoutes.set, availabilitySlot.set); |
There was a problem hiding this comment.
Let's not be verbose... calling it just routes here should be enough. and maybe calling the other one handlers instead of availabilitySlot too.
There was a problem hiding this comment.
Let's write this as a convention. WDYT?
| type ApiRoutesType = Record< | ||
| string, | ||
| { base: ApiRouteBase; routes: Record<string, string> } | ||
| >; |
There was a problem hiding this comment.
Let's call this ApiRoutesMap. WDYT?
| BASE extends ApiBase, | ||
| SUB extends keyof (typeof ApiRoutes)[BASE]["routes"], |
There was a problem hiding this comment.
I don't think TS convention involves writing generics all in capitals letters.
| }); | ||
| } | ||
|
|
||
| generateUrl< |
There was a problem hiding this comment.
It's not a full URL... let call it generatePath, or generateRoutePath.
I love abbreviations like
geninstead ofgenerate😄. However, that's not how we name things in this project.
There was a problem hiding this comment.
I propose exporting all these types and constant in one scope, say ApiRoute. And then renaming each of them accordingly.
ApiRoutesType -> ApiRoutesMap -> ApiRoute.Map
ApiRouteBaseRoute -> ApiRoute.BaseRoute
ApiRoutes -> ApiRoute.Routes
| | "/confirmation-code" | ||
| | "/report"; | ||
|
|
||
| export const ApiRoutes: ApiRoutesType = { |
Quality Checklist
Links