forge42dev / remix-toast

Server side implementation of toast notifications in Remix
MIT License
170 stars 10 forks source link

Toast not working properly #13

Open yarapolana opened 6 months ago

yarapolana commented 6 months ago

Hi I followed the initial steps to get the toast going with sonner see below, but it is not working. Expected behaviour: Toast to work when being called.

What is happening: Toast gets called on refresh twice, and doesn't get cleared, if I refresh it continues to get called.

Is something else missing?

from route

// ... imports

export const action = async ({ request }: ActionFunctionArgs) => {
  return jsonWithError(null, 'There are errors in the form')
}

from root

// ... imports

export const loader = async ({ request }: LoaderFunctionArgs) => {
  // ... otherHeaders logic
  const { toast, headers: toastHeaders } = await getToast(request)

  return json(
    {
      toast
    },
    {
      headers: {
        // ...otherHeaders,
        ...toastHeaders
      }
    }
  )
}

export default function Home() {
  const { toast: serverToast } = useLoaderData<typeof loader>()

  useEffect(() => {
    console.log('serverToast', serverToast)

    if (serverToast?.type === 'success') {
      toast.success(serverToast.message)
    }
    if (serverToast?.type === 'error') {
      toast.error(serverToast.message)
    }
  }, [serverToast])

  return (
    <html lang="en">
      <head>
        <Meta />
        <title>Basic Example With Remix Toast</title>
        <Links />
      </head>
      <body>
        {children}
        <Toaster />
        <ScrollRestoration />
        <Scripts />
      </body>
    </html>
  )
}

using

devDeps
@remix-run/dev -> 2.8.1
@remix-run/eslint-config -> 2.8.1

deps
@remix-run/serve -> 2.8.1
@remix-run/react -> 2.8.1
@remix-run/node -> 2.8.1
sonner -> 1.4.0

Edit: Just wanted to say congrats with the lib though, fantastic explanation on the blog.

AlemTuzlak commented 6 months ago

Hi @yarapolana , thank you so much!

I think the issue has something to do with the fact that you're spreading the toastHeaders like that, headers are not normal JS objects so they don't really act the same, what happens if you return headers: toastHeaders?

yarapolana commented 6 months ago

@AlemTuzlak You are welcome I tried and have the same result, only appears after refresh. I have other headers for auth cookie thats why I had to spread them otherwise auth wouldn't work.

not working

    {
      headers: toastHeaders
    }
kianweelee commented 6 months ago

Hi @yarapolana, I am facing the same issue as well with multiple headers. Do you have a solution for this yet?

AlemTuzlak commented 6 months ago

Do you guys mind creating a minimum repro so I can check it out

AlemTuzlak commented 6 months ago

also would you mind trying using this:

export function combineHeaders(...headers: Array<ResponseInit["headers"] | null | undefined>) {
  const combined = new Headers();
  for (const header of headers) {
    if (!header) {
      continue;
    }
    for (const [key, value] of new Headers(header).entries()) {
      combined.append(key, value);
    }
  }
  return combined;
}

headers: combineHeaders(toastHeaders,otherHeaders)
yarapolana commented 6 months ago

@AlemTuzlak I found out what my issue was.

On the root.tsx file I am using export const shouldRevalidate = () => false if I comment out this line everything works, I used Object.assign, tried combinedHeaders and it also works but only with the line commented out.

Now how can I still use shouldRevalidate and the toast?

GoodluckH commented 5 months ago

Same issue here. Using the latest Remix without shouldRevalidate in the root. Works in dev but has the issue when deployed to production.

wonesy commented 4 months ago

I am also seeing this issue. Always appears twice on refresh and is never evicted from the cookie.

I am also using cookie-based auth, which i am checking prior to getting to this toast stage, so perhaps the issue lies therein

grundmanise commented 1 month ago

Might be related to https://github.com/remix-run/remix/issues/5647#issuecomment-2254685869

AlemTuzlak commented 1 month ago

@grundmanise This might be the actual cause of it but that might be solved when single fetch is released and the API is changed. Unfortunately I can't do much to help without a reliable repro to help figure it out, but if it's caused by a race-condition in Remix there's really not much I could do. I believe you guys are having an issue but I have no idea how to solve it unfortunately 😢