search

epicweb-dev / epic-stack

This is a Full Stack app starter with the foundational things setup and configured for you to hit the ground running on your next EPIC idea.
https://www.epicweb.dev/epic-stack
MIT License
4.88k stars 396 forks source link

Update client-hints implementation to support disabled cookies #280

Closed kentcdodds closed 1 year ago

kentcdodds commented 1 year ago

Not many users do this, but some do and I think (untested) if a user with cookies disabled tried to visit an Epic Stack site they would get infinite refreshes. We should handle this case.

kentcdodds commented 1 year ago

To be clear, if the cookies are disabled, we should use the fallback: https://github.com/epicweb-dev/epic-stack/blob/4db7d7030395e21bc188980315ec1d545bf448c3/app/utils/client-hints.tsx#L13

We just need to figure out how to determine that cookies are disabled.

rajeshdavidbabu commented 1 year ago

I haven't really come across a possible solution yet to reliably identify if cookie is disabled or simply not-set. The behaviour seems to be similar in both cases.

Do you have any preliminary idea around here ? Would navigator.cookieEnabled be an option here ?

kentcdodds commented 1 year ago

I think we could just check that and prevent the reload in that case

rajeshdavidbabu commented 1 year ago

@kentcdodds Sounds good, the fallback will never be used in the client-hints in this case. Do we still need that ?

kentcdodds commented 1 year ago

You're right. Fixed: 4578c7ef7fc526cbe7e7a1e05ed12fb7a0319b5d