Skip to content

fix: reimplement treeshaking non-dynamic prerendered remote functions#15447

Merged
teemingc merged 13 commits into
mainfrom
treeshake-prerendered
Apr 6, 2026
Merged

fix: reimplement treeshaking non-dynamic prerendered remote functions#15447
teemingc merged 13 commits into
mainfrom
treeshake-prerendered

Conversation

@teemingc

@teemingc teemingc commented Feb 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

This PR fixes a regression where prerendered remote functions were no longer treeshaken. It was accidentally removed in https://github.com/sveltejs/kit/pull/14293/changes#diff-6997ee961e0605b37d712e389f6a938a4ae080038f0ed9b9643894b287985feaL51 while we were trying to figure out how to make remote functions in node_modules work correctly.

It also ensures that these prerendered functions can't be called in production with arbitrary inputs. cc: @GauBen

EDIT: In retrospect, this doesn't help remove certain files if they aren’t in use by anyone after the treeshaking occurred.

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

changeset-bot Bot commented Feb 27, 2026 •
edited
Loading

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: dbc2cea

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

vercel[bot]
vercel Bot reviewed Feb 27, 2026
View reviewed changes
Comment thread packages/kit/src/exports/vite/build/remote.js Outdated
@teemingc teemingc marked this pull request as draft March 3, 2026 12:28
@teemingc

teemingc commented Mar 3, 2026

Copy link
Copy Markdown
Member Author

We probably need to fix up any sourcemaps too after the treeshaking

@teemingc teemingc marked this pull request as ready for review March 3, 2026 14:55
@svelte-docs-bot

svelte-docs-bot Bot commented Mar 3, 2026

Copy link
Copy Markdown

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

vercel[bot]
vercel Bot reviewed Mar 3, 2026
View reviewed changes
Comment thread packages/kit/src/exports/vite/build/remote.js
@Rich-Harris

Rich-Harris commented Apr 5, 2026

Copy link
Copy Markdown
Member

Did you investigate whether it's possible to accomplish this by creating a facade that only re-exports the non-statically-prerendered functions and bundles that? If it worked it would allow us to skip all the acorn/magic-string stuff and just lean on Rolldown instead, though if you told me that it doesn't work for some reason then I would certainly believe you

@teemingc

teemingc commented Apr 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Did you investigate whether it's possible to accomplish this by creating a facade that only re-exports the non-statically-prerendered functions and bundles that? If it worked it would allow us to skip all the acorn/magic-string stuff and just lean on Rolldown instead, though if you told me that it doesn't work for some reason then I would certainly believe you

Mm that's what I tried at first with a naive reimplementation of the previous PR. It's tricky because the current output exports an object that contains all the exports including the statically-prerendered ones

const m = /* @__PURE__ */ Object.freeze(/* @__PURE__ */ Object.defineProperty({
  __proto__: null,
  get prerendered() {
    return prerendered;
  },
  get prerendered_entries() {
    return prerendered_entries;
  },
  get with_read() {
    return with_read;
  }
}, Symbol.toStringTag, { value: "Module" }));

I think because that's exported, Rollup's not treeshaking the static ones. And the remote functions themselves are exported individually but aliased with a letter so we can't access them directly

@Rich-Harris

Rich-Harris commented Apr 5, 2026

Copy link
Copy Markdown
Member

Ah yeah that makes total sense!

@teemingc teemingc merged commit 9766d28 into main Apr 6, 2026
28 checks passed
@teemingc teemingc deleted the treeshake-prerendered branch April 6, 2026 07:40
This was referenced Apr 5, 2026
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vercel vercel[bot] vercel[bot] left review comments

@Rich-Harris Rich-Harris Rich-Harris approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@teemingc @Rich-Harris