Skip to content
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

chore: error during development when using use:enhance with +server #13197

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

eltigerchino
Copy link
Member

If we have no plans to extend use:enhance to work with non-SvelteKit form actions then this closes #10855 entirely.

This PR adds an dev-only error if someone uses an enhanced form and POSTs to an internal API handler. It makes it more obvious they shouldn't be used together compared to "Unexpected end of JSON input" when it fails to parse a response body.


Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

  • If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

Edits

  • Please ensure that 'Allow edits from maintainers' is checked. PRs without this option may be closed.

Copy link

changeset-bot bot commented Dec 19, 2024

🦋 Changeset detected

Latest commit: 9400deb

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@sveltejs/kit Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@ITenthusiasm
Copy link

ITenthusiasm commented Dec 19, 2024

If I may, I do think this will leave SvelteKit at a significant disadvantage compared to other frameworks like Remix, which support this use case. (This is particularly/especially true when it comes to things related to auth -- which is a core need in many web applications.)

Would it be possible for enhance to simply support this use case out of the box? Both Remix and Next.js (App Router) use custom headers to detect when redirects need to be handled in a special way. (That is, they use custom headers to allow manual handling of redirects with fetch since currently fetch only allows you to follow redirects automatically.) I even rolled my own version of this logic for the Next.js Pages Router.1 (Other frameworks may have something more sophisticated; I'm not certain.)

Would SvelteKit be able to do something similar? If not, would SvelteKit at least be able to add documentation on how to enhance enhance to support this use case? Since other frameworks support this, and since what I rolled felt pretty simple, I assumed this wouldn't be complicated. (However, I don't know the internals of SvelteKit.)

Footnotes

  1. See also the middleware and response helper that complimented the client-side utility which I linked to.

@eltigerchino
Copy link
Member Author

If I may, I do think this will leave SvelteKit at a significant disadvantage compared to other frameworks like Remix, which support this use case. (This is particularly/especially true when it comes to things related to auth -- which is a core need in many web applications.)

Would it be possible for enhance to simply support this use case out of the box? Both Remix and Next.js (App Router) use custom headers to detect when redirects need to be handled in a special way. (That is, they use custom headers to allow manual handling of redirects with fetch since currently fetch only allows you to follow redirects automatically.) I even rolled my own version of this logic for the Next.js Pages Router.1 (Other frameworks may have something more sophisticated; I'm not certain.)

Would SvelteKit be able to do something similar? If not, would SvelteKit at least be able to add documentation on how to enhance enhance to support this use case? Since other frameworks support this, and since what I rolled felt pretty simple, I assumed this wouldn't be complicated. (However, I don't know the internals of SvelteKit.)

Footnotes

  1. See also the middleware and response helper that complimented the client-side utility which I linked to.

If we do decide to support non-SvelteKit actions, this error should still be helpful in the meantime (until support is added).

@ITenthusiasm
Copy link

If I may, I do think this will leave SvelteKit at a significant disadvantage compared to other frameworks like Remix, which support this use case. (This is particularly/especially true when it comes to things related to auth -- which is a core need in many web applications.)
Would it be possible for enhance to simply support this use case out of the box? Both Remix and Next.js (App Router) use custom headers to detect when redirects need to be handled in a special way. (That is, they use custom headers to allow manual handling of redirects with fetch since currently fetch only allows you to follow redirects automatically.) I even rolled my own version of this logic for the Next.js Pages Router.1 (Other frameworks may have something more sophisticated; I'm not certain.)
Would SvelteKit be able to do something similar? If not, would SvelteKit at least be able to add documentation on how to enhance enhance to support this use case? Since other frameworks support this, and since what I rolled felt pretty simple, I assumed this wouldn't be complicated. (However, I don't know the internals of SvelteKit.)

Footnotes

  1. See also the middleware and response helper that complimented the client-side utility which I linked to.

If we do decide to support non-SvelteKit actions, this error should still be helpful in the meantime (until support is added).

That's fair. If the Svelte team decides that it's worth pursuing this at some point (even if it's in the more-distant future), could #10855 be unlinked from this PR? (I understand that the team may not have made a decision on this yet.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

use:enhance compatibility on form whose action isn't a SvelteKit action
2 participants