feat: allow to specify preload fetch priority #989
Conversation
dargmuesli
force-pushed
the
feat/fetch-priority
branch
from
305b813
to
0842347
Compare
|
@dargmuesli What would be the reason to have this functionality? If you set preload it would most probably mean that you would like the image to have a high priority IMO. Setting it to auto may result in some issues of having a preload and fetchPriority low. If I get this completely wrong, please let me know. Maybe I am missing something :) |
|
Of course the main goal is to be able to specify "high" fetch priority for a preloaded image. I just included "low" and "auto" as it's part of the specification. |
|
@dargmuesli Isnt it set already by default? I thought it is set like that when passing preload. I was looking recently at the unpic library where this is done automatically so maybe I assumed that it is done here as well: If it doesnt work like that, please ignore my previous comment and proceed with thw implementation. Just review the question from Daniel and you should be good to go 😉 Also, I just noticed an issue with the GitHub Actions. Could you check this out as well? |
|
I tried it in Firefox. Using preload only let the request have low priority afair, setting it to high made let it be of high priority. |
|
any resolution for this PR? |
dargmuesli
force-pushed
the
feat/fetch-priority
branch
from
0842347
to
4b3a044
Compare
|
I resolved the merge conflicts. Reviews welcome! |
dargmuesli
force-pushed
the
feat/fetch-priority
branch
from
4b3a044
to
0a09d14
Compare
|
I'm not sure, but to me it looks like the netlify image cdn snapshot need to be updated. If that's incorrect, I'd need assistance to understand how to fix the pipeline instead 🙏 |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #989 +/- ##
==========================================
- Coverage 59.66% 59.63% -0.04%
==========================================
Files 78 78
Lines 5080 5091 +11
Branches 409 409
==========================================
+ Hits 3031 3036 +5
- Misses 2019 2025 +6
Partials 30 30 ☔ View full report in Codecov by Sentry. |
| @@ -94,6 +94,9 @@ export default defineComponent({ | |||
| imagesizes: sizes.value.sizes, | |||
| imagesrcset: sizes.value.srcset, | |||
| }), | |||
| ...(typeof props.preload !== 'boolean' && props.preload.fetchPriority | |||
| ? { fetchpriority: props.preload.fetchPriority } | |||
| : {}), | |||
There was a problem hiding this comment.
should we default to fetchpriority: 'high'?
There was a problem hiding this comment.
Sorry for not answering in time, this comment fell off my radar 🌠
There was a problem hiding this comment.
Still time to update - what do you think?
There was a problem hiding this comment.
Why should the fetch priority default to high in your opinion? I'd say we can just leave it for the users to decide if they want to specify any.
I'd like to be able to set the fetch priority for preloaded images. This is a quick draft of an implementation. Happy to hear feedback ❤️