[chore] use superstruct for config validation

[bluwy]

Addresses #2267 (comment). Better late than never :)

Use superstruct for config validation. Removes old implementation.

Notes

1. Do we want a changeset?
2. The tests are copied from packages/kit/src/core/config/index.spec.js to packages/kit/src/core/config/test/index.js to use the fixtures. I've also removed the hard-coded error message assertions since they're too coupled to the implementation, but lmk if we do want it.
3. packages/kit/src/core/config/options.js is now the single source of truth, even for the types.
4. The error messages are not the same as the previous implementation, but are still readable IMO. e.g.

> [email protected] dev /home/bjorn/Documents/projects/svelte/kit/packages/kit/test/apps/basics
> ../../../svelte-kit.js dev

At path: kit.salad -- Expected a value of type `never`, but received: `"tomato"`
StructError: At path: kit.salad -- Expected a value of type `never`, but received: `"tomato"`

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 pnpx changeset and following the prompts. All changesets should be patch until SvelteKit 1.0

[changeset-bot]

🦋 Changeset detected

Latest commit: ea21d16

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

[benmccann]

nice! I like that a couple hundred lines of code have been removed. this looks good to me

a changeset wouldn't be bad since this is a sizable change eventhough it should be transparent to users. sometimes it's helpful to have big changes in the changelog just in case there's a regression so users can find relevant changes more easily

[bluwy]

Removed the blank lines and added a changeset. Hopefully the config validation is still readable though, I saw astrojs using zod for theirs but the bundle size is a tad larger and non-treeshake-able.

[Rich-Harris]

Thank you, but I'd prefer that we not add new dependencies, honestly — looking at https://unpkg.com/[email protected]/lib/index.es.js, it really seems like overkill for what we need. And the error messages are less friendly (wtf is a StructError, would be my first reaction).

We can DRY the existing code out if it's too verbose — working on a PR.

[bluwy]

Sure, I don't mind if you prefer to improve the existing code instead. Feel free to close this if you believe #2373 is the path forward, no hard feelings :)

Though to add to this PR's motivation, I was hoping to use an established library to prevent creating and testing our own mini-library. I don't think superstruct vs our custom config validation gives a big bundle size margin either since we bundle superstruct and it's treeshaken.

Re errors, yeah maybe it is a bit cryptic for some, that would be a caveat though we could probably re-write the errors.

I'll take a look at #2373 in the meantime and review it.

[Rich-Harris]

It's not just the extra bytes, though they add up quicker than you'd think — dist/cli.js is 40,152 bytes with this PR compared to 27,962 with #2373.

It's that every dependency is an additional possible point of failure, and something that we have to keep updated (but which might introduce new problems with those updates), and something that increases the learning curve for anyone working on the codebase (now they have to go away and learn the API for a new library instead of reading the ~100 LOC that define our version of the same functionality) while reducing flexibility (if we need to change something, we have to hope that it's supported by the library, work around it somehow, or do without).

Robust projects have as few dependencies as possible!

[bluwy]

Thanks for the clarification. The numbers certainly don't lie, perhaps the library's treeshake-able code wasn't as effective as I thought it was. I don't entirely agree with point of failure and learning curve points though since the library is quite well-built and our own implementation has the same learning curve too. But nonetheless I don't want to force this in if it's undesirable, we could circle back to this if we ever need to, so I'll be closing this.