color-mode icon indicating copy to clipboard operation
color-mode copied to clipboard

Cookie storage creates duplicate cookies with path information on nested routes causing conflict

Open Coops0 opened this issue 1 year ago • 10 comments

Version

@nuxtjs/color-mode: 3.5.2 nuxt: 4.0.0-28849585.4751b054 (but this is the same on the latest stable Nuxt v3 version, the repo is on v3 too)

Reproduction Link

github repo

Steps to reproduce

  1. Setup the repo above
  2. Switching the theme on the main page works fine. The cookie persists, and loads properly. (delete the cookie to reset)
  3. Navigate to the nested route, and change the theme. Note in your browser's cookies that it has created 2 cookies with the same name and different paths.
  4. Refresh. This will cause conflicts.

What is Expected?

Cookie persistence to not retain any path information.

What is actually happening?

Image

Coops0 avatar Nov 08 '24 01:11 Coops0

It appears that #301 will resolve this with the ability to hardcode the path value.

Coops0 avatar Nov 08 '24 01:11 Coops0

I concur, currently it saves cookie per route. I doubt that was the intended behavior.

tasiotas avatar Nov 16 '24 23:11 tasiotas

Any updates?

This problem is even worse because server and client handle cookie in different ways. Server uses only root path cookie, client uses current path cookie. It leads to hydration errors.

zumm avatar Jan 21 '25 19:01 zumm

Any updates?

This problem is even worse because server and client handle cookie in different ways. Server uses only root path cookie, client uses current path cookie. It leads to hydration errors.

Here is the solution I have settled with for now. It is not elegant or very good but it is the best I could do quickly. Basically I make a copy of the default theme, but have it as the default for color mode (which you set in the nuxt config). So whenever that specific theme is detected, I try to revert back to the last known USER picked theme, since it is impossible for the user to select the 'fake' default one.

export const useTheme = (userService: UserService | null) => {
    const { user, hasRefreshedRemotely } = useUser(userService);
    const theme = useState('theme', () => user.value?.theme || 'fake_default_theme');

    const activeTheme = useColorMode();

    const setThemeLocal = (name: string) => {
        if (name !== activeTheme.value) {
            activeTheme.value = name;
            activeTheme.preference = name;
        }
    };

    watch(() => activeTheme.value, t => {
        if (t !== 'fake_default_theme') {
            return;
        }

        const userTheme = user.value?.theme;
        if (userTheme) {
            setThemeLocal(userTheme);
        }
    }, { immediate: true });

    watch(theme, setThemeLocal);

    watch(user, p => {
        if (activeTheme.value === 'fake_default_theme' && p) {
            setThemeLocal(p.theme);
        }

        if (hasRefreshedRemotely.value && p?.theme && p.theme !== theme.value) {
            theme.value = p.theme;
            setThemeLocal(p.theme);
        }
    }, { immediate: true, deep: true });

    async function setTheme(name: string) {
        if (name === theme.value) {
            return;
        }

        if (name === 'fake_default_theme') {
            throw new Error('Cannot set theme to placeholder');
        }

        theme.value = name;
        setThemeLocal(name);
        // ... update user remotely 
    }

    return {
        theme: readonly(theme),
        setTheme
    };
};

Coops0 avatar Jan 22 '25 12:01 Coops0

@Coops0 My current solution is a way simpler. I just monkey patched this package. :) Thanks to pnpm patch.

window.document.cookie = storageKey + "=" + preference + "; path=/";

But would be nice to have native solution.

zumm avatar Jan 22 '25 13:01 zumm

@zumm Haha yeah that is a much cleaner solution

Coops0 avatar Jan 22 '25 13:01 Coops0

happy to open a pr @zumm ?

atinux avatar Mar 29 '25 09:03 atinux

happy to open a pr @zumm ?

Shouldn't it be fixed by https://github.com/nuxt-modules/color-mode/pull/301 ?

zumm avatar Mar 29 '25 10:03 zumm

when is this fixed gonna be merged?

ssglopes avatar Apr 27 '25 18:04 ssglopes

This issue still persists, and it seems to be a simple fix. Since last year?

splitwarechef avatar May 16 '25 09:05 splitwarechef