brisa-build / brisa

The Web Platform Framework.
https://brisa.build
MIT License
449 stars 12 forks source link

When the middleware does a redirect the server actions stops working. #577

Closed AlbertSabate closed 1 month ago

AlbertSabate commented 1 month ago

Describe the bug When the middleware does a redirect the server actions stops working.

To Reproduce https://github.com/AlbertSabate/brisa-debug

Expected behavior Server actions should work as expected.

OR

Provide a clear way for executing server actions without passing through the middleware.

aralroca commented 1 month ago

@AlbertSabate how can I reproduce it? I load /about, it redirects to / and then the server button with server action is working fine.

AlbertSabate commented 1 month ago

You have to go /about using the nav menu.

You can see on this video, the server actions seems working, but the middleware intercepts de request and does not work. https://github.com/user-attachments/assets/bcd8c099-a63f-4f19-b371-d2a1b3fd648a

aralroca commented 1 month ago

Okay, I see what's going on. To make the action, it calls the same URL with POST, but it also suffers from the redirect and loses the headers and the body.

Screenshot 2024-10-23 at 19 46 34

I'm going to investigate it. However, I see another issue related. I created here https://github.com/brisa-build/brisa/issues/584

aralroca commented 1 month ago

@AlbertSabate currently is possible to fix the actions from the middleware replacing the redirect to rewrite. This rewrite currently can be done with this:

if(['/about'].includes(req.route.pathname ?? '') && req.headers.has('x-action')) {
  req.finalURL = new URL('/', req.url).toString();
  return 
}

However, maybe we can add a new Brisa API to handle this in a better way. What do you think?ç

About the action detection (req.headers.has('x-action')), this is going to be fixed in this issue https://github.com/brisa-build/brisa/issues/584 with req.renderInitiator, is better since req.headers.has('x-action') may break in the future, the safe way to do it will be through req.renderInitiatior. I'm wondering if rename this API to req.initiator to also add API endpoints here. It will be a getter, so until it is called the value will not be calculated, and it will only be calculated the first time, so it is not a performance cost.

aralroca commented 1 month ago

Actually, what is wrong, is that if you make a redirect, when you navigate with SPA it keep the old url and not the one you have redirected, this is really the cause of the problems with the actions because it is thought to be in another path, which can be solved with the rewrites... But it is a sign that the redirect with SPA is not being managed well. If you have a redirect from /about to -> /, when navigating to /about you should see / to the URL.

I'm fine with adding documentation on how to do the rewrite anyway, although it should be fixed so that the redirect works as expected, and then no further rewrite is needed.

aralroca commented 1 month ago

@AlbertSabate is solved in 0.1.3-canary.5 pre-release 🤗