Remove node-isms from SvelteKit runtime

[Rich-Harris]

This will make it marginally easier to use in environments that don't resemble Node, like Cloudflare Workers and Deno

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

Changesets

• If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpx changeset and following the prompts

[vercel]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/sveltejs/kit-demo/Dw1rZHzXYrZQ3dUCgFbVQKHK9k5o
✅ Preview: https://kit-demo-git-remove-nodeisms-sveltejs1.vercel.app

[Conduitry]

I'm worried about this logic if someone does a same-protocol request. new URL('//foo/bar', 'sveltekit://origin') resolves to 'sveltekit://foo/bar' but shouldn't be treated as an internal fetch.

Perhaps use an unusual domain as well in the base, and also check for that when deciding whether this is an internal fetch?

[Rich-Harris]

Actually it turns out new URL is picky about the second argument; sveltekit://origin is invalid. I think we can use file://sveltekit as a base.

Though I'm not really sure what the correct behaviour would be in the case of a protocol-relative request. Can we even ascertain the protocol that it's relative to? Maybe any path that startsWith('//') should just be an error

[Conduitry]

Huh. new URL('//foo/bar', 'sveltekit://origin') worked for me (Node 15.14.0), and returned:

URL {
  href: 'sveltekit://foo/bar',
  origin: 'null',
  protocol: 'sveltekit:',
  username: '',
  password: '',
  host: 'foo',
  hostname: 'foo',
  port: '',
  pathname: '/bar',
  search: '',
  searchParams: URLSearchParams {},
  hash: ''
}

But yeah, now that you mention it, I'm not sure what a protocol-relative request should do in SSR, since there's no certain way to know what that protocol is. Making it explicitly an error seems to be the only sensible thing.

[Rich-Harris]

It works in Node but fails in the browser. Which is probably fine but if Node ever decides they want to match browser behaviour more closely, it would be a headache. Also, Deno etc might be stricter. Will change it, out of paranoia

[Rich-Harris]

Jesus, Node's URL is completely broken. This expression...

new URL('./bar', 'sveltekit://origin/baz/qux').pathname

equates to //origin/baz/bar in the browser, but /baz/bar in Node.

Screw this, I'm going to find a URL parsing library

[Conduitry]

Isn't /baz/bar what one would expect to be getting? That's the path. The browser behavior is what's surprising me here.

If I do new URL('./bar', 'file://origin/baz/qux').pathname then I do get /baz/bar in the browser as well. I think this is just the browser not handling weird protocols correctly.

[Rich-Harris]

The browser is 'correct' by virtue of being the browser; Node's job is to match its behaviour. I'd be very surprised if browsers aren't behaving according to spec here, even if the spec has surprising outcomes

[Conduitry]

If you're concerned about the browser's behavior being technically correct and about Node's behavior one day switching to match it, I'd then still strongly recommend using a URL with a known protocol that browsers and Node will handle the same way, rather than bringing in a URL parsing library.

[Rich-Harris]

is this still 'changes requested' @Conduitry or are you 👍 on this?

[Conduitry]

Oh, sorry, nope, I'm 👍 now.