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

Merged
merged 4 commits into from
Jan 15, 2025

Conversation

eltigerchino
Copy link
Member

@eltigerchino eltigerchino commented Dec 19, 2024

Related to #10855 (If we have no plans to extend use:enhance to work with non-SvelteKit form actions then this closes that issue 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.)

Copy link
Member

@dummdidumm dummdidumm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's go with this but keep the issue open

@dummdidumm dummdidumm merged commit 00e1a76 into main Jan 15, 2025
14 checks passed
@dummdidumm dummdidumm deleted the use-enhance-api-warning branch January 15, 2025 17:01
@github-actions github-actions bot mentioned this pull request Jan 15, 2025
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.

3 participants