Client Address Bar wont update after 303 redirect response.

MitchMcKean commented 4 months ago

The correct Page will load but the Address Bar is still showing the last Page. At the API the status 303 is used with a new location header.

CAYdenberg commented 4 months ago

Can you show us how you are issuing the redirect and how you are testing it? Because as read this is a description of unexpected behaviour by your browser, not Fresh.

MitchMcKean commented 4 months ago

Yes of course. I hope I am not wasting anyone's time by this maybe being a problem with my browser. So if I hit a page directly by typing the address and the page response is a redirect, everything works fine and maybe I am wrong and this is also expected behavior but if I send a request to the Fresh Server and the response is a redirect, the address bar wont change but the page will load as expected.

So I am trying to use a Form for Login like this

export const handler: Handlers = {
  async GET(req: Request, ctx: FreshContext) {
    return await ctx.render();
  },
  async POST(req: Request, ctx: FreshContext) {
    const form = await req.formData();
    const email = form.get("email")?.toString();
    const password = form.get("password")?.toString();

    console.log(`Login for ${email} with ${password}`);

    const headers = new Headers();
    headers.set("location", "/posts");
    return new Response(null, {
      status: 303,
      headers,
    });
  },
};

export default function Login() {
  return (
    <>
      <Head>
        <title>Login</title>
      </Head>
      <div className="flex flex-wrap w-full max-w-sm bg-emerald-950 rounded-lg mt-24 p-4 mx-auto">
        <h1 className="text-4xl font-extrabold mx-auto">Login</h1>
        <div className="py-4 w-full">
          <form method="POST">
            <label
              className="block text-red-100 text-base uppercase font-bold mb-1"
              htmlFor="email"
            >
              EMail
            </label>
            <input
              type="email"
              id="email"
              name="email"
              className="w-full border bg-gray-800/50 rounded px-3 py-2 outline-none text-white"
            />
            <label
              className="block text-red-100 text-base uppercase font-bold mb-1 mt-4"
              htmlFor="password"
            >
              password
            </label>
            <input
              type="password"
              id="password"
              name="password"
              className="w-full border bg-gray-800/50 rounded px-3 py-2 outline-none text-white"
            />
            <div class="flex w-full justify-center pt-4">
              <button
                type="submit"
                className="relative bg-emerald-500 border border-transparent hover:border-emerald-200 py-2 px-6 rounded-md font-bold text-lg text-white"
              >
                Sign In / Login
              </button>
            </div>
          </form>
        </div>
      </div>
    </>
  );
}

marvinhagemeister commented 4 months ago

Thanks for sharing the reproduction code. That made it a lot easier to understand what your scenario is, what happened and what you did expect to happen. Can confirm that this is a bug.

MitchMcKean commented 3 months ago

could it also be that in this case of a redirect, fresh also does not load any islands that are related to that page that the redirect is pointing to. Only those island exist, that already have been loaded.

saeho commented 3 months ago

I am having the same issue.

Here's a simpler example to help understand.

[/routes/should_redirect_to_hello.ts]

export const handler: Handlers = {
  GET(_req: Request, _ctx: FreshContext) {
    return new Response(null, {
      status: 301,
      headers: {
        location: '/hello',
      },
    });
  },
};

This code will work and also change the browser URL box correctly when you enter this route into the browser.

However if you link to this route from the app by doing:

<a href='/should_redirect_to_hello'>Redirect me</a>

Then this will show the proper page/code from the hello route, but the URL box in browser will say "should_redirect_to_hello" which is wrong.

This is avoidable by making sure the app never uses the "should_redirect_to_hello" link, so technically it's not a big issue. But it's still a bug nonetheless.

In my opinion, it might be better off left the way it is and developers should never use outdated links in their app unless there's a way to check the correctness of the route without an addition of in-app routing layer which will only bloat the framework.

Another option is to use an approach similar to fresh.gen.ts to make sure all links in the app are not broken and not redirects and throw a warning instead. I would like to see the framework keep its simple, non bloated nature.

CAYdenberg commented 2 months ago

I'm not able to reproduce either of these issues (redirect from a link, or from a form submission).

Fresh 1.6.8, tried both Firefox and Chrome. I checked Postman and it looks like all the headers I would expect are present.

I dunno maybe this will help with debugging. Happy to try to get my reproduction case closer to the original issue, if we can.

denoland / fresh

Client Address Bar wont update after 303 redirect response. #2404