benawad / dogehouse

Taking voice conversations to the moon 🚀
https://dogehouse.tv/
MIT License
9.12k stars 1.48k forks source link

fix(kibbeh): redirect with 301 instead of 404 #2829

Closed rocktimsaikia closed 3 years ago

rocktimsaikia commented 3 years ago

fix #2811

The suggestion to completely remove the 404 page and use the next redirect does not actually work. Next.js does not let us grab the 404 redirects like that. There is an open issue on that with a workaround which is currently being used in the Dogehouse frontend too. So that basically ends that primary point on that issue.

But the use of 301 redirects instead of 404 is reasonable. But we can not write the status code with next router. So I ended up using nextjs-redirect which is a zero dependency module that solves this issue with no hassle. By default, it sets the 301 status code so there is no config needed on that.

Another reason to use 301 instead of 404 is that in browsers like Brave it detects the 404 redirects and shows a dialogue box on top of the screen. Using 301 solves that issues.

vercel[bot] commented 3 years ago

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.

storybook – ./kibbeh

🔍 Inspect: https://vercel.com/dogehouse-storybook/storybook/4Lh7JckZH8tafmspD2eo1rtmVJe2
✅ Preview: Canceled

staging – ./kibbeh

🔍 Inspect: https://vercel.com/dogehouse-staging/staging/Ewdk9Rr552CzbwaJDcJqDGV37mpM
✅ Preview: Failed

dogehouse – ./kibbeh

🔍 Inspect: https://vercel.com/benawad/dogehouse/2e7x6NqB8XTfQYRg7WNVJv33VVo8
✅ Preview: https://dogehouse-6w2r6apwr-benawad.vercel.app

amitojsingh366 commented 3 years ago

we don't need an external package for this, and why have you deletedyarn.lock. i'm gonna close this pr as it is unnecessary

rocktimsaikia commented 3 years ago

@amitojsingh366 How do you suggest redirecting with a 301 status code? It's a zero dependency package so I don't think it would have made any performance or build size issue. And I didn't delete the lock I think it was updated because of the newly added package.

Anyway, this issue remains in Brave browser because of the 404 status. Also 404 redirect is misleading since the goal is to not have any 404 pages.

issue

amitojsingh366 commented 3 years ago

@amitojsingh366 How do you suggest redirecting with a 301 status code? It's a zero dependency package so I don't think it would have made any performance or build size issue. And I didn't delete the lock I think it was updated because of the newly added package.

Anyway, this issue remains in Brave browser because of the 404 status. Also 404 redirect is misleading since the goal is to not have any 404 pages.

issue

ill handle it