dustinstacy / nexus-dawn

MIT License
1 stars 0 forks source link

Migrate server dir to TS #53

Closed dustinstacy closed 1 year ago

dustinstacy commented 1 year ago

resolves #11 resolves #12 resolves #13 resolves #14 resolves #15

dustinstacy commented 1 year ago

TS CRAZY!!!

dustinstacy commented 1 year ago

That should be everything inside the server folder. Sure there are some opportunities to improve it somewhere, but it works!

Most glaring opp. I see is the hasUser() inside a couple of the routes. This is a Type guard function (something I had to do a little searching for) to prevent a possibly undefined value. Tried a whole bunch of options to extract this, make it a middleware, .use inside all the routes, and nothing will take except for explicitly calling the check inside each route. Thoughts/Ideas?

andrew-fleming commented 1 year ago

Most glaring opp. I see is the hasUser() inside a couple of the routes. This is a Type guard function (something I had to do a little searching for) to prevent a possibly undefined value. Tried a whole bunch of options to extract this, make it a middleware, .use inside all the routes, and nothing will take except for explicitly calling the check inside each route. Thoughts/Ideas?

Hmm I unfortunately do not. Assuming we have to keep the hasUser, would it be easier to abstract the check into a function e.g.

router.get('/', requiresAuth, async (req, res, next) => {
    try {
        if (!hasUser(req)) {
            return res.status(404).json({ error: 'User not found' })
        }
    ...
}

into something like:

router.get('/', requiresAuth, async (req, res, next) => {
    try {
        checkUser(req)
    ...
}
dustinstacy commented 1 year ago

HAHAHA, that was very entertaining as I scrolled through there.

Surprised that didn't provoke an angry compiler? I'll go ahead and get those squared away!

dustinstacy commented 1 year ago

Hmm I unfortunately do not. Assuming we have to keep the hasUser, would it be easier to abstract the check into a function e.g.

That was my first guess as well and for some reason TS throws a fit when it's formatted like that. Maybe I was missing a small detail so I'll give it another go since this seems like the obvious answer. Also, since it's used across multiple scripts I think a utils folder is in order.

andrew-fleming commented 1 year ago

Surprised that didn't provoke an angry compiler?

Yeah, that's weird. There were red squigglies on my machine when I looked at those files

andrew-fleming commented 1 year ago

That was my first guess as well and for some reason TS throws a fit when it's formatted like that. Maybe I was missing a small detail so I'll give it another go since this seems like the obvious answer.

Worst case, we can always come back to it later. I don't think it's worth stalling development over this

andrew-fleming commented 1 year ago

since it's used across multiple scripts I think a utils folder is in order.

Yes yes yes

dustinstacy commented 1 year ago

hasUser has been extracted into /utils and working as expected 👍