feat: allow to specify preload fetch priority

[dargmuesli]

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 ❤️

[Baroshem]

@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 :)

[dargmuesli]

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.

[Baroshem]

@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?

[Baroshem]

Some comments from my side

[dargmuesli]

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.

[Javid-Izadfar]

any resolution for this PR?

[dargmuesli]

I resolved the merge conflicts. Reviews welcome!

[dargmuesli]

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-commenter]

Codecov Report

Attention: Patch coverage is 50.00000% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 59.63%. Comparing base (5262730) to head (f63fc2a).

Files	Patch %	Lines
src/runtime/components/nuxt-img.ts	0.00%	3 Missing ⚠️
src/runtime/components/nuxt-picture.ts	0.00%	3 Missing ⚠️

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.
📢 Have feedback on the report? Share it here.

[danielroe]

should we default to fetchpriority: 'high'?

[dargmuesli]

Sorry for not answering in time, this comment fell off my radar 🌠

[danielroe]

Still time to update - what do you think?

[dargmuesli]

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.

[ausir0726]

@dargmuesli
Thank you for the epic contribution, I came here early following the update log.
I recently encountered an issue with download priority.
Although the popularity of fetchPriority is not high, it has already solved most problems based on the UA currently observed.

Also, I would like to ask why fetchPriority needs to be placed separately in the preload object?
Because writing it this way makes the code relatively difficult to maintain,
or does preload have other expected extended functions?

According to my wish, the first setting of the carousel should be set to high as an example

// v-for
:preload="{ fetchPriority: index == 0 ? 'high' : 'low'}"

If simply using fetchPriority as an attribute

// v-for
:fetchPriority="index == 0 ? 'high' : 'low'"

[dargmuesli]

I think I implemented it that way because Nuxt image includes preload link addition to the document's head. Having done a quick research, I think that it's possible to set the fetchPriority on the image element only. So a PR adding fetchPriority as an additional property might be a good idea if that's true. But it wouldn't replace the current implementation. It would only correct it so that the preload's fetchPriority would be set on the preload link element only.

[ausir0726]

@dargmuesli
That's a great idea.
Additionally, I noticed that you mentioned fetchPriority in your writing, which could possibly be a boolean.
Perhaps when the boolean is true, it can be set to high, and when false, it can remain unchanged.
If fetchPriority is a string, it can provide the user's desired operation, achieving this.

// v-for
:fetchPriority="index == 0"
// Expecting to fetchpriority="high"
// else do nothing ( not low or auto ) , do not add any links to the head

[dargmuesli]

Hmm, I'd favor a string as there can be the "enum" values high, low and auto. I see where you're coming from with the true=high suggestion, but that assumes that people understand fetch priority as a tool to increase the priority, but I think people could also want to use it to lower an image's priority.

[ausir0726]

Hmm, I'd favor a string as there can be the "enum" values high, low and auto. I see where you're coming from with the true=high suggestion, but that assumes that people understand fetch priority as a tool to increase the priority, but I think people could also want to use it to lower an image's priority.

Indeed, based on the literal interpretation, fetchPriority should mean to increase or decrease.
So initially, before having any documentation, I thought the preload parameter could be directly set to true.
It should then directly return fetchpriority="high", but after reading the source code, I realised it had to be non-boolean and include fetchPriority.
Of course, this is just an idea to enhance usability.
I don't have any problems with the current usage 👍.
After all, only a few images affecting user's first visit would require this setting.

Asking about design patterns, on the one hand, is for learning and referencing design 😄