Client-side routing for <form method="GET">

[Rich-Harris]

Closes #7251. Just a failing test for now, because we need to refactor the find_anchor stuff to make this work, and there's no point embarking on that until #7776 is merged.

TODO:

• add a submit handler (roughly described here, except it needs an event.defaultPrevented check)
• add a form navigation type (or submit? not sure), that is applied regardless of willUnload
• docs

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. All changesets should be patch until SvelteKit 1.0

[changeset-bot]

🦋 Changeset detected

Latest commit: 2536be1

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

[tcc-sejohnson]

Also opening another (very small) PR against this for type improvements

[tcc-sejohnson]

Suggested change

	/** @type {HTMLFormElement} */ (event.target)
	/** @type {HTMLFormElement | null} */ (event.target)

again, probably shouldn't pretend nullable things aren't possibly null and should just handle null in the function or return early

[Rich-Harris]

this is just TypeScript being dumb though, no? Could event.target ever actually be null?

[dummdidumm]

It's not null in this case though, we know it exists. The types don't, that's why they play it safe.

[tcc-sejohnson]

Opened #7883 for smol type improvements

[PatrickG]

Suggested change

	export function find_anchor(element, target) {
	while (element !== target) {
	if (element.nodeName.toUpperCase() === 'A') {
	return /** @type {HTMLAnchorElement | SVGAElement} */ (element);
	}
	element = /** @type {Element} */ (parent_element(element));
	}
	}
	export function find_anchor(element, target) {
	if (element.nodeName.toUpperCase() !== 'A') {
	element = element.closest('a');
	}
	if (element && target.contains(element)) {
	return /** @type {HTMLAnchorElement | SVGAElement} */ (element);
	}
	}

I'm not sure if this is considered premature optimization already, but I thought it might be worth a suggestion.
I'm not a benchmark expert, but I created this (probably unrealistic) benchmark which hints that closest + contains (which we need anyway) might be faster than the while loop:
https://measurethat.net/Benchmarks/Show/22320/0/find-anchor

[Rich-Harris]

I've no doubt that's true, but unfortunately closest() doesn't work, because it can't find elements in parent shadow DOM — yet another reason adding it to the web platform was a mistake

[Rich-Harris]

(incidentally, element.closest(selector) will return element if it matches selector, so if this did work — if we decided not to bother with shadow DOM, on the basis that the wounds are self-inflicted — we could dispense with the first if block)

[PatrickG]

I've no doubt that's true, but unfortunately closest() doesn't work, because it can't find elements in parent shadow DOM — yet another reason adding it to the web platform was a mistake

Oh, makes sense. I didn't thought about that.

(incidentally, element.closest(selector) will return element if it matches selector, so if this did work — if we decided not to bother with shadow DOM, on the basis that the wounds are self-inflicted — we could dispense with the first if block)

I included this element.nodeName.toUpperCase() !== 'A' check because it was so much faster than .closest('a') on an HTMLAnchorElement in my tests.

[PatrickG]

Suggested change

if (external || options.reload) continue;

This is a remnant of my previous suggestion. Github didn't let me include this line in the selection because there was a deleted line in between.

[ignatiusmb]

Thanks, we also need to delete some of the empty line or lint would fail, removed in a separate commit.