Upgrade SolidStart to v1 #35
Conversation
ERmilburn02
left a comment
ERmilburn02
left a comment
There was a problem hiding this comment.
I mostly just have some issues with some changes to the packages that didn't need to be changed
There was a problem hiding this comment.
Why was Typescript added as a dependency to the root project?
| "@lucia-auth/adapter-sqlite": "^3.0.1", | ||
| "@solidjs/router": "^0.13.3", | ||
| "@solidjs/start": "^1.0.0", | ||
| "@types/better-sqlite3": "^7.6.10", |
There was a problem hiding this comment.
Why were types moved to regular dependencies?
| "@lucia-auth/adapter-sqlite": "^3.0.1", | ||
| "@solidjs/router": "^0.13.3", | ||
| "@solidjs/start": "^1.0.0", | ||
| "@types/better-sqlite3": "^7.6.10", |
There was a problem hiding this comment.
Again, why are types in regular dependencies? They're not required at runtime, only during development.
| return sendRedirect(event, "/"); | ||
| const cookie = lucia.createSessionCookie(session.id); | ||
|
|
||
| setCookie(cookie.name, cookie.value, cookie.attributes); |
There was a problem hiding this comment.
You should be returning here, otherwise you throw an error because you try to add the user to the database again
| const cookie = lucia.createSessionCookie(session.id); | ||
|
|
||
| setCookie(cookie.name, cookie.value, cookie.attributes); | ||
| return Response.redirect("/"); |
There was a problem hiding this comment.
This is throwing a type error because it can't parse the URL. Instead you should probably use redirect from @solidjs/router
|
@ERmilburn02 I read the comments in this PR, saw that it hasn't been merged, and created a new PR that fixes the problem, but also does the minimum changes in order to fix the code to bring it up to date with the |
4 participants
No description provided.