feat: make client router treeshakeable #11340
Conversation
This removes the `create_client` method in `client.js` in favor of having everything as exported functions at the top level. Slight rearrangements to make some things lazy or put it behind a `BROWSER ?` condition are necessary, but otherwise the code is almost completely untouched. This makes it even less likely we can ever unit test the client router, but I think that ship has sailed a long time ago and e2e tests are a much more robust and an outright better way to test this, so it's a non-issue.
🦋 Changeset detectedLatest commit: 706db8a 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 |
|
Mhm the test failure is legit, since there are two apps loaded embedded-style which share the same global stuff that's overriding each other. It was added due to #9576 - although I'm wondering if the test that was added really reflects what happens in the real world. In the NYT case these are probably separate SvelteKit apps. I don't see how they wouldn't clobber each other as soon as you use Therefore, I'm leaning towards going ahead with the change people who inline multiple SvelteKit apps into their site will most likely create separate bundles. Are there other ways to enforce separate instances? |
…ng a query string
|
POC for potential solution: force deduplication of modules by appending a query string when detecting embedded mode. Ideally each embedding has a unique but stable ID - no idea how that could look like. |
|
Linking to #9808 just to double check if that wouldn't happen again (I don't think so) |
use an increasing id -> things are still cached, but dynamically, and Vite plays ball, too
Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
|
@dummdidumm heads up that there's a merge conflict on this now |
* chore: apply state to history.state directly The serialization capabilities of history.state are basically identical to those of devalue.stringify . As such, the indirection of saving the data in the session storage isn't necessary, which also saves 1kb compressed / 3kb uncompressed because of the no longer needed devalue functions * Apply suggestions from code review --------- Co-authored-by: Rich Harris <richard.a.harris@gmail.com>
* reduce indirection * fix * tweak error messages * sigh --------- Co-authored-by: Rich Harris <rich.harris@vercel.com>
* remove all the embedded hackery * ugh --------- Co-authored-by: Rich Harris <rich.harris@vercel.com>
|
Fantastic work. I think this'll probably unlock some broader improvements in how the code is organised but we can look into those later |
This removes the
create_clientmethod inclient.jsin favor of having everything as exported functions at the top level. Slight rearrangements to make some things lazy or put it behind aBROWSER ?condition are necessary, but otherwise the code is almost completely untouched. This makes it even less likely we can ever unit test the client router, but I think that ship has sailed a long time ago and e2e tests are a much more robust and an outright better way to test this, so it's a non-issue.A hello world SvelteKit app shrinks by 2 kilobytes (uncompressed) from doing this.
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:.