-
-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
use:enhance
compatibility on form whose action isn't a SvelteKit action
#10855
Comments
use:enhance
on form whose action isn't a SvelteKit actionuse:enhance
compatibility on form whose action isn't a SvelteKit action
strange that when i downloaded the code and used it inside |
Yes! Can we please go with the option of "Alternatively, we could handle the non-JSON responses better" (mentioned in the StackBlitz link). For me, this actually is somewhat urgent, not a nice-to-have. Here's the background of my use case: I'm working with an open source authentication solution called Note: The The ScenarioImagine a user is on a page that displays new data on the current screen depending on what they submit to the server. This is a perfect use case for Where SvelteKit WorksImagine a non-JS user is on this webpage. They submit a form with an expired Access Token and a valid Refresh Token. (Note that all tokens are expressed as HTTP-only cookies.) Here's what will happen:
Where SvelteKit FailsNow imagine a JS user is on this webpage. They submit a form with an expired Access Token and an expired Refresh Token. Here's what will happen:
|
I've run into an issue today with redirect form actions and setting headers. I thought form actions would work like post server endpoint and returning a response with redirect settings would redirect but no get an error because form actions only expects data to return and using set headers redirect doesn't keep headers. So it would be nice even with actions to be able to return redirect responses that are not data to or from form. |
Do you mind elaborating on this with a code example? |
I can make a repo later and share, no problem. |
@JonathonRP is the issue that you're talking about similar to what's shown in the StackBlitz link in the OP? Or is it something different? |
@ITenthusiasm similar, I'm using form actions with use enhance and also not working if I return a response that doesn't have data prop or is sveltekit redirect(), I'm attempting to have redirect and settings authorization header. |
@eltigerchino, repo - |
I know this has been marked as non-urgent, but I still consider this a pretty valuable feature. It's more or less necessary for an authenticated application as I described earlier. Would it be possible for SvelteKit to simply read the response type (or some custom headers) to see if it should navigate to the provided URL instead of trying to unwrap some JSON? I believe Remix does something like this. |
We documented our findings in `NOTES.md` at: ITenthusiasm/remix-supertokens@2726466 NOTE: Due to limitations in SvelteKit, there are some tests that won't work. Specifically, any scenario which tests that an HTML page is returned in response to a form submission (with `use:enhance`) will fail. (This can happen, for example, if a user is redirected to a login page during form submission because their session expired.) Other frameworks like Remix do not have this problem. The Svelte team seems to be aware of this issue, though its priority is unknown. Track sveltejs/kit#10855 for additional details.
What a pain in the proverbial this has been today....
<form method="post" action="/logout" use:enhance>
<button>Sign out</button>
</form>
export const POST: RequestHandler = (event: RequestEvent) => {
// do the needful
return new Response(null, {
status: 302,
headers: {
Location: "/"
}
});
};
Another oddity, I shall make a repro for this, if you have
<a href="/login/google">Sign in with Google</a>
export const GET: RequestHandler = (event: RequestEvent) => {
// do the needful
return new Response(null, {
status: 302,
headers: {
Location: "/"
}
});
}; When you click the link you get a nice little
Which comes from here presumably because the route does not exist /as a page/ |
https://stackblitz.com/edit/sveltejs-kit-template-default-ipxflmtx Here's an up to date repro of both |
Wouldn't it work if you implemented it as a page action instead of using
The only thing you should return from a SvelteKit form action is a plain object or optionally using the See https://svelte.dev/docs/kit/form-actions for more information.
You can still set the headers using import { redirect } from "@sveltejs/kit";
export const actions = {
default: async ({ setHeaders }) => {
setHeaders({ token: "1" });
redirect(303, "/dashboard");
},
};
It's tricky. A redirect causes the fetch to respond with the result of fetching the location header's URL rather than the original endpoint so we can't just check the status code. We could try checking the request URL against the action URL and if it's different, we've probably been redirected, but it feels somewhat hacky: if (response.url !== action.toString()) {
result = { type: 'redirect', status: response.status, location: response.url };
} We'd need to add this check in between the fetch and the deserialise: kit/packages/kit/src/runtime/app/forms.js Lines 190 to 198 in c0ca073
|
Describe the problem
An unhelpful error message presents itself when we have a form with
use:enhance
posting to an ordinary POST endpoint instead of a SvelteKit action. This occurs because the enhance action tries to parse the fetch response body as a JSON. If that succeeds, it looks for adata
property and attempts to parse it as JSON too, which can throw an error. If there is nodata
property, submitting the form results in nothing happening, which can be confusing.https://stackblitz.com/edit/sveltejs-kit-template-default-nmxvlc?file=src%2Froutes%2F%2Blayout.svelte
Describe the proposed solution
Maybe we can handle the outcome better or have a dev-only warning when this occurs?
Alternatives considered
Document that
use:enhance
should only be used with SvelteKit actionsImportance
nice to have
Additional Information
No response
The text was updated successfully, but these errors were encountered: