feat: add preprocessor warning about common mistakes

[dummdidumm]

Uses a preprocessor to check the script contents of Svelte files for "export const prerender/csr/ssr/trailingsSlash =" or a missing <slot /> in case of a layout file
closes #7411
closes #7773

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

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	Minor

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

[dominikg]

you could add this via plugin.api.sveltePreprocess https://github.com/sveltejs/vite-plugin-svelte/blob/main/docs/faq.md#how-do-i-add-a-svelte-preprocessor-from-a-vite-plugin i think.

Is this safe though or are there false positives? Maybe only do it for dev mode?

[dummdidumm]

Would it make a difference in practice? What would be the advantage of providing it through plugin.api-sveltePreprocess (especially given that apparently there's some config option to turn it off)?

The slot check is definitely safe, if anything it would give false negatives. The prerender thing could theoretically result in false positives if someone commented out the code, but why would you have something like that commented out.

Making it dev only isn't much use, this warning is only spitted out at build time (which could be seen as a good thing), it has no effect on runtime.

[dominikg]

can preprocess return warnings or do you think it's possible to use onwarn here? leaving users no way out of this log feels bad.

or is this something for an eslint plugin sveltekit? squiggles would be even more helpful.

[benmccann]

I have mixed feelings about warning people for page options in .svelte files. I'm assuming it's mostly people upgrading from older versions. We removed all our other upgrade / deprecation warnings with the release of 1.0, so it feels a bit out of place to add it and I'm guessing adding a preprocessor can have some performance implications

[benmccann]

should also mention missing slot in layout or make this changeset more generic to cover both cases

[Rich-Harris]

Yeah, I have the same reservations. For the page options, I think the cost of adding a preprocessor outweighs the benefit of helping a tiny minority of developers who (frankly) didn't read the docs.

For slots, you get this error at dev time:

<Layout> received an unexpected slot "default".

Perhaps we could capture that warning and replace it with an error like

+layout.svelte components must contain a <slot />

We're already intercepting warnings here:

kit/packages/kit/src/runtime/client/client.js

Lines 1727 to 1741 in b6ca02a

	if (DEV) {
	// Nasty hack to silence harmless warnings the user can do nothing about
	const console_warn = console.warn;
	console.warn = function warn(...args) {
	if (
	args.length === 1 &&
	/<(Layout|Page)(_[\w$]+)?> was created (with unknown|without expected) prop '(data|form)'/.test(
	args[0]
	)
	) {
	return;
	}
	console_warn(...args);
	};
	}

[dummdidumm]

I don't share the argument of that preprocessor slowing things down - since only +page/layout files are analyzed, we are talking about single digit miliseconds for the whole build here.
I also don't share the "tiny minority didn't read the docs" sentiment. This issue has come up in many forms since the rework, also from people who didn't know the old way previously. I'd rather have clear warnings that guide new people than having to answer the same issues/Discord threads over and over again.
I would be ok with moving this warning to the browser console - frankly I'm not sure which terminal is likely to be read more. In the case of the static adapter it's unlikely though, since the flow to get into this is probably "ok I build this, oh there's an error telling me to set export const prerender = true I'll add this to +layout.svelte, mhm it still doesn't work".
I will also add something to language tools itself so people using VS Code get the info right in their editor - still something inside Kit itself is needed for those who don't use that IDE.

[Rich-Harris]

Fair enough. I'm less concerned about performance than I am about adding more stuff to the codebase, especially something that feels so disconnected from all other compiler warnings/errors. And warnings were doubling up because files are preprocessed twice (once for server, once for client) — short of getting fancy, the only way to prevent that duplication is to prevent the same warning from being printed multiple times, which further reduces the likelihood of someone seeing the message at the right time during dev. (Though that was already a problem; it would only show up when the file is first transformed or immediately after it was edited).

A dev runtime check would be much better, if we could get the error into the overlay, but we don't currently have a way to inspect components (we can check for context="module" exports but that's about it).

So I don't love this, and think we should remove it as soon as a better option becomes available (I don't consider this sort of thing subject to semver constraints) but in the absence of one today we'll add it for the sake of closing those issues