Conversation
🦋 Changeset detectedLatest commit: 75b93d9 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 |
|
@bluwy what should we do to test it on Firefox? I can help |
|
We probably need to update We also need to update this to install the firefox etc binaries: Lines 33 to 34 in 1c59059 |
|
Awesome, I can make a PR for that if you're happy |
| } | ||
| // Prefetch with link if supported | ||
| else if ( | ||
| document.createElement('link').relList?.supports?.('prefetch') && |
There was a problem hiding this comment.
Isn't it somewhat expensive to do this on every link? Could it be done once?
There was a problem hiding this comment.
I benchmarked this locally and there isn't a big difference in perf from what I can tell compared to caching, so I kept this direct approach, and to save some bytes.
bholmesdev
left a comment
There was a problem hiding this comment.
The conditional checks look right to me. Glad we're getting with the times!
|
I added Firefox to the e2e tests, however some of the tests fail e.g. view transitions |
Changes
Closes #10464
<link rel="prefetch">if supported, or else fall back tofetch().prefetch()API, unless thewith: 'link' | 'fetch'option is passed. Thewithoption is deprecated as it's better to use link whenever possible.More explanation for why this change is made at #10464 (comment)
Testing
The existing tests should pass. Testing is a bit tricky because this mainly matters for Firefox and Safari but we don't have those setup. I think manual testing should be enough for now as the prefetch code is isolated and doesn't changed often.
Docs
withastro/docs#8246