[fix] handle lone returns in migration tool, turn on strict mode

[dummdidumm]

Adding strict mode uncovered that we previously failed on lone return; statements. This PR fixes that - but it brought up an interesting question: Should lone return; statements still be allowed in the new load function? Right now it's implemented as "no, you need to adjust this". If we allow it, we should adjust the types for the handler functions in +page.server.js to allow it as well. If we don't allow it, we should adjust the load function return type to reflect hat.

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: 4bdfeda

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
svelte-migrate	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

[Rich-Harris]

what if we changed the implementation of rewrite_returns instead? I don't think there's any reason we'd need to do anything with a return;, we can just leave it be:

-if (ts.isReturnStatement(node)) {
+if (ts.isReturnStatement(node) && node.expression) {

[dummdidumm]

Depends on how we answer the questions in the PR description - so yeah, definitely possible

[Rich-Harris]

At present the type of Load is this:

kit/packages/kit/types/index.d.ts

Lines 180 to 186 in ac5b00b

	export interface Load<
	Params extends Record<string, string> = Record<string, string>,
	InputData extends Record<string, any> = Record<string, any>,
	OutputData extends Record<string, any> = Record<string, any>
	> {
	(event: LoadEvent<Params, InputData>): MaybePromise<OutputData | void>;
	}

i.e. a void return is fine, and so assuming we're okay with that then return; is fine

[dummdidumm]

Right, but currently the RequestHandler type does not allow a void return - which makes me think that we should probably align these. So Load is ok with a void, so GET should, too. In the case of standalone endpoints we always need to return a response now. If we assume that people have their returns correctly already, we can just skip the return; case - if you agree with that, I'll adjust the code accordingly.

[Rich-Harris]

If standalone endpoint returns void today, it'll be an error, so I think it's fine for RequestHandler to require a Response return value — it'll just continue to error if it doesn't

[dummdidumm]

True - ok I'll adjust this to leave lone returns as-is then