Conversation
| import { getTosRequirementForUser } from "~/models/tos.server"; | ||
| import { getUserId } from "~/utils/session.server"; | ||
|
|
||
| function isAllowedUiPath(pathname: string) { |
There was a problem hiding this comment.
Is there any reason why we couldn't handle both ui and server in one middleware?
They seem very related to each other and technically we are always handling just "requests", right?
I do understand the response types are different, but this way there also seems to be quite a bit of duplication?
Maybe I am just too picky though :D
There was a problem hiding this comment.
Yes i thought about it as well, i initially thought that maybe in a single file it could get a bit messy with potentially more branches over time, but thinking about it again i guess this would still be quite manageable. In terms of code duplication i think the way it is at the moment is still somewhat justifiable, but i think i could still add a small helper to further reduce it.
There was a problem hiding this comment.
I am also thinking a bit about "mental load" here. People have to remember to put the right middleware for the right kind of route. It's not like its hugely complicated though, so keeping it separate is probably perfectly fine.
Having more time comparing the two, I am realizing where the differences are. It does make it more complicated to unify the two, so overall we would be trading that for complexity of the code.
I'd rather create a base class/ function for each type of route to make sure the right middleware is called automatically and people don't have to bear the mental load of differentiating ;-)
There was a problem hiding this comment.
The right middleware should already be called automatically due to where they are attached (ui middlware in root.tsx and api in routes.api). Is this what you meant?
2 participants
Type of Change
Implementation
Checklist
devbranchAdditional Information