remove all the embedded hackery #11545
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Rich-Harris
added a commit
that referenced
this pull request
Jan 8, 2024
* feat: make client router treeshakeable 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 * fix PURE annotation positions * POC for potential solution: force deduplication of modules by appending a query string * refine embedded approach use an increasing id -> things are still cached, but dynamically, and Vite plays ball, too * hacking my way to victory * tweak * move whole startup logic into create_client * these can be private functions now * fix * Update packages/kit/src/runtime/client/start_embedded.js Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com> * chore: apply state to history.state directly (#11521) * 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 in treeshakeable client (#11544) * reduce indirection * fix * tweak error messages * sigh --------- Co-authored-by: Rich Harris <rich.harris@vercel.com> * remove all the embedded hackery (#11545) * remove all the embedded hackery * ugh --------- Co-authored-by: Rich Harris <rich.harris@vercel.com> * reduce diff size --------- Co-authored-by: Rich Harris <rich.harris@vercel.com> Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com> Co-authored-by: Rich Harris <richard.a.harris@gmail.com>
dummdidumm
added a commit
that referenced
this pull request
Jan 9, 2024
5 tasks
Rich-Harris
pushed a commit
that referenced
this pull request
Jan 9, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
start_embedded.jsstuff feels a bit odd to me — we never needed to load multiple copies ofclient.jsbefore, and I don't really see why the changes in #11340 would mean that we need to now. AFAICT all we need to do to get the tests passing as before is to ensure that we hydrate the correct element.There's still a bunch of quirks around embedded mode (which IIRC is primarily meant to allow you to embed a single SvelteKit app in a larger app, i.e. it prevents you from triggering the host app's own router — not to allow multiple embeds on a single page) but those are unrelated to the current work and so can live for another day.