typos + title #568
typos + title #568
Conversation
app/routes/_index.tsx
Outdated
| export const loader = async ({request}: Parameters<LoaderFunction>[0]) => { | ||
| const url = new URL(request.url) | ||
| const stateFromUrl = url.searchParams.get('state') | ||
| if (stateFromUrl) { | ||
| const firstOpenId = getStateEntries(stateFromUrl).filter( | ||
| ([_, state]) => state === QuestionState.OPEN | ||
| )[0]?.[0] | ||
| if (firstOpenId) { | ||
| url.searchParams.delete('state') | ||
| url.pathname = questionUrl({pageid: firstOpenId}) | ||
| throw redirect(url.toString()) | ||
| } | ||
| } | ||
| return null | ||
| } |
There was a problem hiding this comment.
@mruwnik what's rationale on removing this?
I think this code is providing backwards compatibility for links from the old site to go to the question that the link points to. Even though the new site is live now, couldn't the links still be out there?
Of course, maybe supporting this feature isn't worth it or maybe the implementation is causing problems.
There was a problem hiding this comment.
no, you're right. I thought this was dead code, but of course it's required to support old links
| data.then((val) => { | ||
| const question = val as Question | ||
| if (question.title) document.title = question.title | ||
| }) |
There was a problem hiding this comment.
I think ideally this as Question cast would be removed as it breaks type safety and makes it more likely that a bug would be introduced if the return type of the loader is changed. This seems better IMHO:
| data.then((val) => { | |
| const question = val as Question | |
| if (question.title) document.title = question.title | |
| }) | |
| data.then((val) => { | |
| if ('title' in val && val.title) document.title = val.title | |
| }) |
If a property is used in the conditional that doesn't help with the narrowing, then there is a TS error
If a property is used in the assignment that doesn't exist on the narrowed type, then there is also a TS error
No description provided.