fix: allow calling fetch for any scheme

[danielniccoli]

Fixes #10657

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:.

[changeset-bot]

🦋 Changeset detected

Latest commit: 5cba1a2

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

[danielniccoli]

Continuation of #10656

That sounds fine, but I think your translation from text to reflex omitted uppercase letters and "+"

Originally posted by @benmccann in #10656 (comment)

The "+" is in there, hidden in plain sight. \d+ does match on digits and plus because its within square brackets.

The uppercase letters are also matched due to insensitive flag.

[benmccann]

Thanks. This looks good to me, but could we refactor this shared logic into a util method like is_relative_url probably in src/utils/url.js?

[danielniccoli]

@benmccann, I have put some thought into your suggestion and I think it would be better to not have a function and accept that this regex will be used at two places. The test answers the questions "Is this a relative URL?" with an approximation that is just good enough for the specific case. For example, it will tell you that "mailto:[email protected]" is an absolute URL. But it's a URI and neither absolute nor relative. (Which we can ignore because a developer wouldn't do this.) Putting this into a function may encourage devs to use it elsewhere, which could cause unintentional side effects. Of course, we could do proper validation and cover all the edge cases as well, but that would introduce unnecessary complexity.

I do not have an alternative suggestion. I added some comments to the two lines to clarify what is tested and why.

Let me know your thoughts. If you think the concerns are negligible, I will refactor.

[benmccann]

That's a fair point. How about extracting the regex out into a constant like SCHEME_REGEX in utils/url.js or something along those lines?

[danielniccoli]

@benmccann I like that. There's already a scheme variable declared, but it's not exported, so I think we can get away with an additional exported constant? (I don't want to rewrite the already present one, because I don't know what resolve() does and where it's used.)

import { BROWSER } from 'esm-env';

// <<<<<<<<<<<<<<<
/** @constant - Matches a URI scheme (https://www.rfc-editor.org/rfc/rfc3986#section-3.1)
    @type {RegExp}
    @default
*/
export const SCHEME = /^[a-z][a-z\d+\-.]+:/i;
// >>>>>>>>>>>>>>>

const absolute = /^([a-z]+:)?\/?\//;
const scheme = /^[a-z]+:/;

/**
 * @param {string} base
 * @param {string} path
 */
export function resolve(base, path) {

[benmccann]

Just go ahead and update the existing one. resolve is used during prerendering. I don't think anything should be negatively affected by updating the existing scheme regex with a more accurate one

[danielniccoli]

Refactored.

[benmccann]

Thanks for the great PR!