feat: add signal property to request in vite, node environments #13895
Conversation
🦋 Changeset detectedLatest commit: 30d6bda The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
|
Oops! This is a change to kit core too, not just the node adapter. 🤦 |
orisrightpaw
changed the title
@sveltejs/adapter-node
| @@ -0,0 +1,6 @@ | |||
| --- | |||
| '@sveltejs/adapter-node': minor | |||
There was a problem hiding this comment.
this doesn't actually touch the adapter-node code at all
| '@sveltejs/adapter-node': minor |
There was a problem hiding this comment.
It does touch @sveltejs/kit/node, though, and the Node adapter is bundled, so we need to re-build it and release a new version.
There was a problem hiding this comment.
On that note, @sveltejs/kit/node's getRequest is also used in local development, in preview mode, and in production in the Vercel adapter. Is this going to have any impact on those?
There was a problem hiding this comment.
On that note,
@sveltejs/kit/node'sgetRequestis also used in local development, inpreviewmode, and in production in the Vercel adapter. Is this going to have any impact on those?
Yes, it would. Apologies for not being clearer. Though, I wasn't aware that the vercel adapter used getRequest. I'll mark @sveltejs/adapter-vercel for a minor version, as well.
There was a problem hiding this comment.
adapter-vercel does use it, but it's not bundled, so probably doesn't need to be listed here
|
this would need some documentation. possibly in the |
orisrightpaw
changed the title
Alright. I'll see what I can do. |
orisrightpaw
force-pushed
the
request-abortsignal
branch
from
12ff4b5 to
2654850
Compare
|
After a bit of testing behavior in Vercel, I couldn't get the request to hang up early without a body. It works fine on POST requests with data, but will not work on GET requests. I guess it's just a Vercel thing...? Someone else might be able to get it to work, but from what I can see, I don't think Vercel will tell you when the client cancels the request. I've removed the Vercel package from the changeset. On another note though, I squashed my commits and wrote some documentation. It's under the |
orisrightpaw
changed the title
Co-authored-by: Ben McCann <[email protected]>
orisrightpaw
force-pushed
the
request-abortsignal
branch
from
adb11bb to
30d6bda
Compare
|
It looks like this should also fix an issue I'm having. I made a quick example using the ORPC sandbox example. It's setup to build a production environment just to avoid any dev specific issues: https://stackblitz.com/edit/github-2qmfa8p9?file=src%2Frouter%2Fsse.ts,src%2Froutes%2F%2Bpage.svelte,src%2Froutes%2Frpc%2F[...rest]%2F%2Bserver.ts Just refreshing the page increases the count of connections, without any cleanup. In that example I didn't try this pull request but I did on my local project, and despite the This is a pretty important feature so I'm glad to see work is being done here to improve it. |
|
Is there any way to turn this into a test? |
Gives routes an AbortSignal (as present in the Request object) to know when the other end hangs up prematurely.
Fixes #12823 for
vite dev+previewand@sveltejs/adapter-nodePlease don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm testand lint the project withpnpm lintandpnpm checkChangesets
pnpm changesetand following the prompts. Changesets that add features should beminorand those that fix bugs should bepatch. Please prefix changeset messages withfeat:,fix:, orchore:.Edits