Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[breaking] only route pages on the client-side #2656

Merged
merged 3 commits into from
Oct 23, 2021
Merged

Conversation

benmccann
Copy link
Member

@benmccann benmccann commented Oct 21, 2021

Right now we have to send a list of all endpoints to the client, but 99% of the time they aren't used for anything and so it's generally unnecessary bandwidth overhead.

Removing this means that developers need to put rel="external" on any link to an endpoint. This seems like a reasonable trade-off since it is very rare to link to endpoints. Endpoints are primarily gotten via fetch which doesn't utilize the router.

In addition to reducing the size of the client JS, it also will make it slightly easier to refactor out the router (#2611) since it will have one less concern to deal with

@changeset-bot
Copy link

changeset-bot bot commented Oct 21, 2021

🦋 Changeset detected

Latest commit: efd5fb7

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

@benmccann benmccann force-pushed the csr-pages-only branch 5 times, most recently from 2101f8d to 91476ca Compare October 21, 2021 05:20
@Conduitry
Copy link
Member

Discussions about sveltekit:external aside, I do think this is probably a reasonable change to make to how routing is handled if it means less stuff getting sent to the client. Rich should probably weigh in, though, and if we're going to be making this change, we should make sure the docs are updated to make the new behavior clear.

@benmccann
Copy link
Member Author

Asked Rich about this on Discord:

snuck a look — i think the rel=external thing is a reasonable tradeoff

@benmccann benmccann merged commit e4e37a2 into master Oct 23, 2021
@benmccann benmccann deleted the csr-pages-only branch October 23, 2021 16:54
@bekos
Copy link

bekos commented Oct 27, 2021

@benmccann Is there an equivalent option when using goto instead of href?

@benmccann
Copy link
Member Author

you could just do document.location.href = newUrl; instead of using goto

@bekos
Copy link

bekos commented Oct 27, 2021

thx for the tip @benmccann 👍 the only issue is that I lost the $navigating changes and it's loading animation in this case 🤷but I can live with that 😃

@benquan
Copy link

benquan commented Sep 29, 2023

This reasoning is completely ommited from the documentation. This post is the only reference I have found to do this. The client side error doesn't help either:
Screenshot 2023-09-29 at 12 14 40 PM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants