Refactor/tidy server #1322
Refactor/tidy server #1322
Conversation
alectrocute
requested review from
dessalines and
SleeplessOne1917
as code owners
| out["x-real-ip"] = realIp as string; | ||
| } | ||
|
|
||
| const forwardedFor = headers["x-forwarded-for"]; |
There was a problem hiding this comment.
Possible bug here I found during this refactor. headers["x-forwarded-for"] can be either string or string[]. Just FYI. If you change the as to satisfies in this file, it becomes apparent.
There was a problem hiding this comment.
Hrm... I know lemmy-js-client just takes in headers?: { [key: string]: string, so maybe any coercion isn't really necessary.
There was a problem hiding this comment.
Thx for doing this, looks much cleaner.
@SleeplessOne1917 and I are getting fairly antsy to start yet another lemmy-ui in leptos-rs, with tailwind + DaisyUI , and that will avoid this entire inferno isomorphic mess, since it already has first-class isomorphic support. But that's for a later time.
| out["x-real-ip"] = realIp as string; | ||
| } | ||
|
|
||
| const forwardedFor = headers["x-forwarded-for"]; |
There was a problem hiding this comment.
Hrm... I know lemmy-js-client just takes in headers?: { [key: string]: string, so maybe any coercion isn't really necessary.
I can't wait to get involved with that. That's an awesome stack. |
|
I would 100% support people charging ahead on that without me. Right now I'm just a bit overwhelmed (800+ of the important notifications, many more unimportant), and am only responding to PRs and coding. |
|
I'll take a closer look at this later, but be aware that the changes in #1043 will likely affect this. |
That's totally fine, it'll be easy for me to resolve the conflicts. Thanks for the heads up! |
|
@dessalines Daisy UI may not even be necessary. We can define our own reusable classes using |
|
@alectrocute #1043 is merged now. |
|
@SleeplessOne1917 I think CI is stuck. :( |
|
I moved our CI to a different server, hopefully that should be okay now. |
|
I fixed the merge conflicts introduced by #1043, this PR should be good to merge! |
|
I have no idea why CI is failing on the merged changes. I'm not seeing any type errors locally. Hmm. |
|
@alectrocute I'm not sure why CI isn't working unfortunately. It's complaining about code that doesn't exist anymore. |
Hi Lemdevs!
I propose we:
server/index.tsxto use a common Express.js pattern (handlers,middlewareandutils)In future:
CatchAllHandlerto be simpler if possibleThanks so much!