Repository navigation
fix: reimplement treeshaking non-dynamic prerendered remote functions - #15447
Conversation
🦋 Changeset detectedLatest commit: dbc2cea The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
Co-authored-by: vercel[bot] <35613825+vercel[bot]@users.noreply.github.com>
… into treeshake-prerendered
|
We probably need to fix up any sourcemaps too after the treeshaking |
|
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 |
|
Ah yeah that makes total sense! |
Two bugs in `treeshake_prerendered_remotes`. Rolldown keeps `chunk.name` as the source basename even when it deduplicates output `fileName`s, so with several `data.remote.js` files in an app they all match the first `data.remote` chunk (since #15447): that chunk gets stubbed repeatedly while the real ones ship their prerender bodies live. The async app already has this collision, which the new test uses. The `return` in the rebuild loop (ba614e6) fires when `build.sourcemap` is set, because the output then also contains the map asset, and every chunk after the first is left stubbed but not re-treeshaken. No test for this half since it needs a user-set `build.sourcemap`; verified by instrumenting a `--sourcemap` build before and after. --- ### 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. - [x] Ideally, include a test that fails without this PR but passes with it. ### Tests - [x] Run the tests with `pnpm test` and lint the project with `pnpm lint` and `pnpm 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 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 - [x] Please ensure that 'Allow edits from maintainers' is checked. PRs without this option may be closed. --------- Co-authored-by: Rich Harris <richard.a.harris@gmail.com> Co-authored-by: Rich Harris <rich.harris@vercel.com>
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:
Tests
pnpm testand lint the project withpnpm lintandpnpm checkChangesets
pnpm changesetand following the prompts. Changesets that add features should beminorand those that fix bugs should bepatch. Please prefix changeset messages withfeat:,fix:, orchore:.Edits