feat: lazy discovery of remote functions

[Rich-Harris]

Closes #13979.

We want to be able to use remote functions in node_modules or wherever else they may be imported from. Right now that's difficult, because we need to know about them in advance — they're part of the app's manifest that's generated by walking directories in src.

It's not practical to traverse the entirety of node_modules — it can grow to be a huge mess of deeply nested folders and recursive symlinks. It's much better to just find them as a by-product of bundling. The challenge is that on the server we need to be able to make them importable by themselves, since if the server receives a request for /_app/remote/xyz123/blah it needs to be able to run the function in question, and Rollup may have bundled things in a way that makes that impossible.

This PR solves it in two steps. First, we use the manualChunks option to force Rollup to create one chunk per remote function (and another for $app/server, since otherwise it'll all go in the chunk for the first remote function it happens to discover, and all other remote modules will have to go via that one — that'll still be the case for many shared dependencies, so it's not a perfect solution, but it's better than nothing).

Second, we prevent those functions from being treeshaken, and export them as the default, so that SvelteKit can get a reference to them via a side entrance.

There's a separate question about whether we need to make remote-functions-in-node-modules opt-in, but there's no point trying to resolve that unless we solve the lazy discovery problem.

---

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

Edits

• Please ensure that 'Allow edits from maintainers' is checked. PRs without this option may be closed.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 1a92bd9

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[svelte-docs-bot]

Preview: https://svelte-dev-git-preview-kit-14293-svelte.vercel.app/

[Rich-Harris]

This is currently failing because imports from .remote.ts files are being treeshaken in the server build (for example if they're only referenced inside an event handler). Not entirely sure how to force them to be exposed

[Rich-Harris]

Okay, I think this is working now. (CI is still running, so... we'll see.) The implementation I settled on: we add the augmentation/validation directly inside the transform hook instead of generating a facade module, and we always use a self-import.

The real trick here is that when building, we add an $$_export_$$($$_self_$$) call, which prevents anything from being treeshaken. When bundling is complete, we can simply replace that with export default $$_self_$$.

This means that while modules that directly import from a remote file use the named exports, as you would expect, SvelteKit itself grabs the default export, which is an object containing the remote functions. This turns out to be a fairly simple approach.

One trade-off: we no longer have the ability to treeshake prerender functions. I think I'm probably okay with this — it was always a bit of a fiction (it only worked to one 'level'), so while we're losing a little bit of optimisation I'm not sure it'll make a big difference in practice.

[ottomated]

there's no point trying to resolve that unless we solve the lazy discovery problem

If the path to the remote functions can be determined from the opt-in method (for example, if you specify a list of package names, those could be resolved to paths), then lazy discovery isn't needed, right?

[dummdidumm]

One trade-off: we no longer have the ability to treeshake prerender functions

We should validate this with the svelte.dev repo. I believe we do some file reading etc at build time which is then all tree shaken and it would fail at runtime. But I'm not sure it would only fail on access or right on startup

[Rich-Harris]

If the path to the remote functions can be determined from the opt-in method (for example, if you specify a list of package names, those could be resolved to paths), then lazy discovery isn't needed, right?

Right, #14156 is definitely a possible approach (thank you!). And I definitely wouldn't rule it out. I just want to be sure that if we use something like allowedPaths it's because we've decided that it's right to do so, and not because we're forced into it by technical limitations.

The downsides are:

• you have to configure it! (other things being equal, configuration should be avoided)
• there's still the potential to do a bunch of unnecessary traversal (both within src and node_modules/some-library) — honestly probably not a huge deal but every millisecond counts on startup
• your API options are limited — you have to use strings. you can't have a regex or a (id) => boolean function

Letting external packages define remote functions is definitely better DX (and, not for nothing, a capability that the React ecosystem doesn't currently have IIUC). And I'm sympathetic to the view that if you use a library in your app you are trusting it not to do anything malicious, in which case allow-listing shouldn't be necessary. I'm just also open to the counterargument that it represents a slightly larger surface area.

[eltigerchino]

Is it worth it at this point to think about adding a check for Rolldown and using the advancedChunks option instead of manualChunks? https://vite.dev/guide/rolldown#manualchunks-to-advancedchunks

[Rich-Harris]

At some point, yeah