-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
enhance(prefetch): Adds a 'load'
prefetch strategy, and ignores 3g
in slow connection detection
#9513
Conversation
🦋 Changeset detectedLatest commit: aa3ee52 The changes in this PR will be included in the next version bump. 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 |
I'm not really sure we should add this option. How would the value be inaccurate? If we detect the user is on slow connection or data saver mode, it would not be a good practice to ignore that and prefetch anyways.
Thanks for adding this. It's something I held off adding before since I figured there isn't a big usecase, but I don't mind if someone adds it. Naming-wise maybe
I'm a bit worried that |
@bluwy Thanks for your reply. I have renamed
|
No need to apologize! And thanks for updating them quickly. Re your points:
|
|
Hello, @bluwy I tested {
downlink: 1.3,
effectiveType: "3g",
onchange: null,
rtt: 850,
saveData: false
} Or sometimes "4g". But at the same time, I also tested the internet speed on https://fast.com/ and https://www.speedtest.net/, and the result was 86Mbps. So I think the results of the I'm not sure if the result of |
I think |
@bluwy Hello, I have resubmitted the code.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I pushed a commit to clean up the changesets a bit. The rest looks great! Thanks for implementing the feature. Since this includes a feature, we'll release this together in the next minor, which should come around the next couple weeks when the rest return from the holidays.
At the meantime, feel free to also update the prefetch docs about the new load
strategy at https://github.com/withastro/docs/edit/main/src/content/docs/en/guides/prefetch.mdx
'load'
prefetch strategy, and ignores 3g
in slow connection detection
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bluwy does this PR require some changes in the docs?
Yeah it's updated at withastro/docs#5967 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For visibility, docs is approving the changeset!
Changes
ignoreSlowConnection
in prefetch config.Because the result of
navigator.connection.effectiveType
is inaccurate, or unstable, users are allowed to globally ignore this judgment.defaultStrategy
inprefetch
config can beall
.Allow users to preload all links on the current page by default.
Testing
I have add a e2e test in
packages\astro\e2e\prefetch.test.js
, namedPrefetch (prefetchAll: true, defaultStrategy: 'all')
Docs
defaultStrategy
to be'all'
.The comments in code:
/cc @withastro/maintainers-docs for feedback!