[bug] formAction$ behaves differently on redirect than globalAction$

[tuurbo]

When you submit with a formAction$, the routeLoader$ fires and then fires again after the redirect. But, if you redirect from within qwik's globalAction$ instead, routeLoader correctly only fires once (after the redirect).

I think the problem is formAction$ shouldn't fire all loaders again before the redirect.

steps to reproduce: https://github.com/tuurbo/qwik-form-actions

qwik form (correct behavior)

• click login link
• click "login (qwik)" button
• should see below in server console

  loader /
  loader /login/
  formLoader
  action (qwik)
  loader /q-data.json

modular form (bad behavior)

• clear server console
• click login link
• click "login (modular)" button
• should see below in server console

  loader /
  loader /login/
  formLoader
  action (modular)
  RedirectMessage {}             <----- not sure why this is logged
  loader /login/q-data.json      <----- this should not be called
  formLoader                     <----- this should not be called
  loader /q-data.json

[fabian-hiller]

Thank you very much for your attention. Do you have any idea what I can do differently? formAction$ builds on globalAction$ and adds nothing in this regard. Here is the code: https://github.com/fabian-hiller/modular-forms/blob/main/packages/qwik/src/actions/formAction%24.ts

[tuurbo]

I found the culprit, but I'm not exactly sure how to fix it. When you redirect in qwik, you prepend the event.redirect() with throw. (eg: throw event.redirect(302, '/')).

The throw is being caught by this try/catch statement. Which explains why RedirectMessage {} is logged to the console. https://github.com/fabian-hiller/modular-forms/blob/main/packages/qwik/src/actions/formAction%24.ts#L117

Maybe in the catch we could rethrow the error, eg:

if (error instanceof AbortMessage) {
  throw error;
}

Here are the AbortMessage and RedirectMessage Qwik classes. They don't seem to be public though. https://github.com/BuilderIO/qwik/blob/main/packages/qwik-city/middleware/request-handler/redirect-handler.ts

Maybe you know another way to go about it?

[fabian-hiller]

Thanks a lot for your research! I will look into it and fix it in the next few days.

[fabian-hiller]

As soon as I get feedback from Qwik on my issue, I will work out a solution to this problem.

[fabian-hiller]

I created a PR to speed up the process: https://github.com/BuilderIO/qwik/pull/4077

[fabian-hiller]

My PR has been merged. As soon as the new Qwik version is available, I will add the if statement.

[fabian-hiller]

The new version is available. 🎉

[tuurbo]

Thanks!

Note: It doesn't work in dev but does work in prod. If I manually edit the modular form code in node_modules and insert console.log(error instanceof AbortMessage); it returns false in dev but in prod it returns true. Not sure anything can be done about that but I wanted to note it in case someone else runs into this issue.

[fabian-hiller]

I have the problem in dev too. However, I assume that this is not a technical problem with the code, but with the package manager. I assume that the AbortMessage class exists multiple times and is not obtained from the same source in each case. Because of this instanceof AbortMessage does not work.

[juanpmarin]

@fabian-hiller is there a way to make this work on dev? I'm not being able to do redirections

[juanpmarin]

It is failling with latest version of qwik, v1.2.5 Both in dev and prod

[fabian-hiller]

Thank you for the hint. I will investigate this.

[fabian-hiller]

@juanpmarin in Qwik v.1.1.5 both works for me. Dev and preview. In Qwik v1.2.5 only dev works. With preview I get an error in the console. If I upgrade Modular Forms Qwik to v1.2.5 it behaves the same, except that I get a different error in the console. I recommend downgrading your Qwik version until we find a solution for the problem. I suspect it is a Qwik issue and not a Modular Forms issue. Feel free to create an issue in the Qwik repo with the errors you are getting and link this issue.

[juanpmarin]

@fabian-hiller do you know about a workaround for this?

[fabian-hiller]

Downgrade to Qwik v1.1.5

[fabian-hiller]

I have the problem. That's why I created this issue. Until this is fixed, I recommend downgrading to v1.1.5.

[fabian-hiller]

It occurs when you use the formAction$ API of Modular Forms. However, I strongly believe that Qwik is the problem and that we as a library cannot do anything.

[fabian-hiller]

Problem with Qwik 1.2.X should be fixed in v0.20.0 of Modular Forms. Make sure, this two imports are imported in the following order in entry.preview.tsx:

import qwikCityPlan from "@qwik-city-plan"; 
import render from "./entry.ssr";

[juanpmarin]

@fabian-hiller are you sure that fix is for this issue? I've upgraded qwik and modular forms and redirection still does not work

[fabian-hiller]

@juanpmarin my comment was related to the previous comments on the Qwik 1.2.6 problem. I will fix the problem you describe soon. In production it should work normally.

[juanpmarin]

Hi @fabian-hiller ! I would love to upgrade qwik to the latest version, do you have any clues about why redirection is not working? Maybe I can do some research

[fabian-hiller]

Does it not work in production either? This could be a possible fix for dev: https://github.com/fabian-hiller/modular-forms/pull/66

[juanpmarin]

@fabian-hiller that fix makes sense to me, it could even stay enabled in production, what do you think?

[fabian-hiller]

I would only activate it in dev. In prod it has no use. I will release a new version with the change in the next weeks. If you want to speed up the process, you can revise the PR or create a new one.

[juanpmarin]

@fabian-hiller redirects are not working for me in production either, I suspect qwik-speak is causing the error, because it duplicates the bundle for every language, does that make sense to you?

[fabian-hiller]

Does it work for your without qwik-speak? Which Qwik version are you using?

[juanpmarin]

Hi @fabian-hiller I did some more tests, I tried to upgrade my project to the latest Qwik and modular forms version again, redirect still does not work but for a different reason I think, look:

I found that when I submit the form, a 404 status code is received:

I tried to look for that action id in the server bundle and I found a similar one, only the prefix changes from id_ to s_ If I fix the id in the URL, and try to execute the same request with curl, I get a success response

Original path: /quotes/new/q-data.json?qaction=id_AW14dmUcJXw Modified path: /quotes/new/q-data.json?qaction=s_AW14dmUcJXw

Do you have any idea of why is not the right id being sent?

[fabian-hiller]

If you don't need any of the new features, I recommend downgrading Qwik. Currently I have very little time, so I don't know when I can look at the problem. Probably it is a problem on the Qwik's side. Feel free to create an issue there and link me.

[juanpmarin]

@fabian-hiller I understand, I'm trying to make it easier for you. Upgrading qwik version is starting to become a priority for us as we're waiting for this fix in upcoming releases, because it represents a security risk.

After a lot of debugging, I found that the 404 error code is thrown when I redirect to a relative URL, like redirect(302, "../test"), what is weird is that it worked in previous versions of qwik, so I'll try to reproduce it without modular forms

[juanpmarin]

@fabian-hiller I was finally able to create a minimal repro, the error only happens with formAction$, routeAction$ redirects as expected

https://github.com/juanpmarin/qwik-issues-repro/blob/9926dc6b457f5cdc5dafc839aed33f89904d37e3/src/routes/relative-redirect/index.tsx#L23-L26

[fabian-hiller]

I understand. As long as the problem can be solved by changes to Modular Forms, I am very interested in implementing those changes directly. Since formAction$ is not extremely complicated and basically just builds on globalAction$, it should be quick once I know what to do.