ciscoheat / sveltekit-flash-message

Send temporary data after redirect, usually from endpoints. Works with both SSR and client.
https://www.npmjs.com/package/sveltekit-flash-message
MIT License
246 stars 5 forks source link

Problems when subscribing to flash store and hovering over <a> tag where load function uses redirect. #17

Closed hansaskov closed 11 months ago

hansaskov commented 11 months ago

Issue Description

I am currently using sveltekit-flash-message to show a toast after redirecting from a protected page to a public page. To achieve this, I followed the example in the readme by subscribing to the flash store, which triggers my toast each time the redirect happens. However, I encountered an issue where the toast sometimes triggers even when I redirect to a page where the flash-message redirect is not used. I have checked for the undefined case, but the problem persists.

Investigation

After some investigation, I found a way to reproduce the error. The issue occurs when I hover over the protected button, where the flash-message redirect occurs on load. Then, if I click another link, the flash-message gets set again, and the toast re-triggers.

Reproduction Steps

To reproduce this issue, follow these steps:

  1. Go to the public page.
  2. Hover over the protected button (where the flash-message redirect happens on load).
  3. Go to the homepage.

Then the toast is unnecessarily shown.

Try it yourself

Go and run my example, just pnpm iand pnpm dev to run it https://github.com/hansaskov/sveltekit-flash-messages-toasts-error

ciscoheat commented 11 months ago

Since Svelte preloads the links, this is expected. You can modify that behavior with these options: https://kit.svelte.dev/docs/link-options

hansaskov commented 11 months ago

Thank you. You are correct that changing the data-sveltekit-preload-data attribute to be tap instead of hover does fix this issue.

Do you think it is possible to not set the flash-message if the preload page is not navigated to?

ciscoheat commented 11 months ago

Not unless there is a way to detect if a link is preloading through the RequestEvent, and I don't think there is.

hansaskov commented 11 months ago

I have figured out a workaround, that i believe makes sense. The idea is to store the url pathname we expect to navigate to within the flash-message, then only trigger the Toast if the expected url pathname matches the pages pathname.

Example

Start by updating the flash message to include the expected pathname we navigate to.

// app.d.ts
declare namespace App {
    interface PageData {
        flash?: {
            type: 'success' | 'error';
            message: string;
            pathname: string;  // Update here
        };
    }
}

When we try to navigate to the /privatepage, we redirected to the homepage with the message of "Message from /private" as well as the pathname of the page we expect to redirect to.

// src/routes/private/+page.server.ts
export const load: PageServerLoad = async (event) => {
    const message = { type: 'error', message: 'Message from /private', pathname: '/' } as const;
    throw redirect(303, message.pathname, message, event);
};

In the layout page we subscribe to the flash store, which will trigger each time the flash-message is updated. The code will make an early return if $flash is undefined or if the pathname of the current page is not the same as the expected pathname from the flash-message

// layout.svelte
import { getFlash } from 'sveltekit-flash-message/client';
import { page } from '$app/stores';

const flash = getFlash(page);

flash.subscribe(($flash) => {
    if (!$flash) return;
    if ($flash.pathname != $page.url.pathname) return;

    errorToast.message = $flash.message;
    toastStore.trigger(errorToast);

});;

You might have noticed that a small bug is still present with the above implementation. When hovering over the private page, then manually navigating to the homepage, the toast is incorrectly displayed. This can be fixed by only triggering the toast if the navigation was done using redirects. To implement this we can create the isGotoNavigated variable and subscribe it to the after navigate store to be true when the navigation was of type goto. Then return early if the navigation is not a redirect.

import { getFlash } from 'sveltekit-flash-message/client';
import { page } from '$app/stores';
import { afterNavigate } from '$app/navigation';

const flash = getFlash(page);

let isGotoNavigated = false;
afterNavigate(({type}) => {
    isGotoNavigated = ['goto'].includes(type as string); // is true when the navigation was a redirect
})

flash.subscribe(($flash) => {
    if (!$flash) return;
    if ($flash.pathname != $page.url.pathname) return;
    if (!isGotoNavigated) return;

    errorToast.message = $flash.message;
    toastStore.trigger(errorToast);
});

I would really love to see this implemented directly into the library as i believe it represents what most people expect when using the it for the first time.

I would like to hear your feedback on this, and if there are some downsides to this implementation. I also would like to contribute myself and put it under a settings flag. But that will be in a couple of days

ciscoheat commented 11 months ago

I'd like to keep things simple, so introducing an extra variable into the flash message isn't feasible as a requirement. Testing the type in afterNavigate isn't really optimal either since the cookie has already been set during the preload.

The only way to keep things simple would be to prevent the redirect (and hence the cookie) during the preload, but if it's not possible to detect preload in the load function, I'd rather add a note about data-sveltekit-preload-data in the docs.

hansaskov commented 11 months ago

That is totally understandable, preventing the redirect is the most elegant solution. And i think you are correct that differentiating between preloaded and non preloaded functions is non trivial.

I'm closing this issue, as it is intended behavior that can be mentioned in the docs.

ciscoheat commented 11 months ago

Thanks for understanding, I'll reopen this until I've fixed the docs!