fix: ensure `preloadCode` without arguments preloads all routes
fixes #8469
This was possible previously, and the function signature indicated that this is possible, therefore marked this as a fix
Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
- [x] 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
- [x] This message body should clearly illustrate what problems it solves.
- [ ] Ideally, include a test that fails without this PR but passes with it.
Tests
- [x] Run the tests with
pnpm testand lint the project withpnpm lintandpnpm check
Changesets
- [x] If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running
pnpm changesetand following the prompts. Changesets that add features should beminorand those that fix bugs should bepatch. Please prefix changeset messages withfeat:,fix:, orchore:.
🦋 Changeset detected
Latest commit: 3e03b0bea3ce04da73def268dc73835dea05833e
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
This was a deliberate change — preloadCode(...paths) having totally different behaviour if paths happens to be an empty array as opposed to an array with a single member is a really weird API. It's also a footgun — on larger sites, this could be a huge amount of code that gets downloaded and executed (and can never be evicted from the module cache). This is what service workers are for.
Open to debate, of course, but I'm 👎 on this
The issue this would close states that this is a small site where it's benefitial to just load everything in one go. Angular has a built-in behavior that does the same by toggling a boolean. I wouldn't want people to having to got out of their way to achieve this, "use a service worker" is not a good workaround IMO.
Given that I want some form of this to land - how could the API look like? Personally loading this without arguments makes intuitively sense to me, and people who accidentally do this - well...
If you really don't want the "no argument" case, what about using * as a reserved string to signal this? preloadCode('*') can't match any routes (has to begin with /) so it's safe to use, and it's similar to kit.prerender.entries.
* could work. Though maybe /* already does? Not sure if wildcards are limited to a single segment. An alternative could be a separate API like preloadAllCode, similar to invalidate vs invalidateAll, though I think I prefer the other ideas.
Regardless, it is still a footgun. I would be happier if we simply demonstrated how to achieve it with a service worker - it's not difficult. 'Angular does it' is not a test we should be using :)
Everything's a footgun at some point, I think we should allow this for people who want to opt into it. /* wouldn't work because it matches the string against the route id regex, so that would only catch /[stuff] at the top level.
I'm going to implement the * idea.
If we do ultimately decide to go ahead with this (which I don't have especially strong opinions about), we need to make sure we update the PR title and the changeset.
good catch — updated the PR title/changeset