Support preloading of fonts in the libraries API

I understand that for now, given the example, it is to add the possibility to specify additional "things" with the library that would generate the link tags. What about the existing external / js that are added in a library?
Should we do something like:

my_library:
  js:
   https://cdn.example.com/js/file.js: { type: external, preconnect: true, dns-prefech: true }

or

my_library:
  js:
   https://cdn.example.com/js/file.js: { type: external }
  links:
    -
      rel: preconnect
      href: //cdn.example.com
    -
      rel: dsn-prefetch
      href: //cdn.example.com
      

The second option allows a lot of flexibility (and cover the example of fonts) but is much more verbose.
Third option is a mix, allowing for shorter version for elements of the library (like example 1) and verbose version for other elements.

I am curious of a direction as we are for now implementing custom on a project but would be keen to write a patch for this to land in core the proper way.

@catch thanks for the redirect to this issue. As I posted on the other issue, in my mind I think a definition like this would be nice;

font:
  css:
    base:
      https://fonts.example.com/font.css: { type: external }
  preconnect:
    https://fonts.example.com: {}
  preload:
    https://fonts.example.com/font-regular.woff2: { as: font, attributes: { type: font/woff2} }
    https://fonts.example.com/font-bold.woff2: { as: font, attributes: { type: font/woff2 } }

dns-prefetch would also have it's own entry. I think having separate entries instead of one generic 'links' entry reduces the risk of user error (and general misuse - eg putting css and js into the links entry). It's also clearer to read.

Ahem, since the other issue was opened in 2022 and this one in 2023, the duplicate is this one, not the former...
We should have close that issue and improve the original.

The other issue has been postponed for a year, and no one seemed to care... I don't mind which issue so long as it stays active.

DNS prefetch is more widely supported and recommended as a fallback for Preconnect (see support), so maybe we can build that in? Or instead of two separate entries it's something like;

font:
  prefetch:
    https://fonts.example.com: { preconnect: true }
    https://other.example.com: { }
  preload:
     etc..

To optionally add a preconnect hint for certain dns-prefetch items, since preconnect usage should be limited to only critical assets;

<link rel="preconnect" href="http://fonts.example.com">
<link rel="dns-prefetch" href="http://fonts.example.com">
<link rel="dns-prefetch" href="http://other.example.com">

Regarding preload, there's also modulepreload to consider. I don't know if we want to have yet another entry just for this separate rel value or if we could so something like;

main:
  js:
    js/main.module.js: { attributes: { type: module } }
    js/main.js: { }
  preload:
    js/main.module.js: { as: script, attributes: { type: module } }
    js/main.js: { as: script }

And if the preload has type:module we can swap the rel value over? Or just as:module?

<link rel="preload" href="main.js" as="script" />
<link rel="modulepreload" href="main.module.js" />

And then how should we handle the crossorigin attribute? Currently it can be added via attributes which I guess is fine.

I don't really mind as well it's just about people who spent hours thinking about the problem / trying to fix it / reviewing code / rerolling it and who will never get credit for that.

As long as the subject moves forward it's good, so now that it's been done, let's continue here.

It would be so nice if we could manage preloading from libraries directly.

(Interaction Observer IO API and Preloading API) in Drupal Core

Not caught up, just one note on fonts.

We added something like this (just preload, not early hints) to https://drupal.org/project/bs_base, by adding preload fonts to theme.info.yml. One tedious bit was cache busting, we for example have an icon font that updates pretty frequently and because this isn't the canonical font definition (that's in CSS), it's very easy for the actual font-face in CSS and this preload info to get out of sync, resulting in loading two different files. That doesn't just defeat the whole purpose, it makes it worse than not having it at all.

I'm not sure what a good solution for this is to be honest, in our case we then added support for updating the .info.yml definition into the icon font build toolchain. I think if core respects the library version and adds that to the URL, similar to CSS/JS assets then that's probably OK and we could update our toolchain to maintain the library version for us?

Yes, that's my point. The two URL's must be the same, the CSS URL would need to use the same version string. I guess if the following definition works then that's also OK?

  https://fonts.example.com/font-regular.woff2?version=5:
    preload: TRUE
    early_hint: TRUE

I'm just saying that this is something that you need to keep in mind, it must be the exact same URL, and if you have it in two different places, it's easy to forget to update them if it's not automated.

1. Fixed phpcs
2. Added support for Navigation

---

We need to fix path resolving
http://localhost:8888/node/add/core/modules/navigation/assets/fonts/inte...

on inner pages it is wrong.

This one needs a rebase, but based on lauriii comment should a CR be added? Or IS updated?

The Needs Review Queue Bot tested this issue. The merge request has merge conflicts and cannot be merged. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Hello everyone,

Thanks all for the discussions and work on this issue!

This looks promising!

I put some review comments on the MR.

Thanks @catch for pushing this.

I change to needs work as last pipeline failed.

Otherwise RTBC for me.

Thanks.

I gave a last quick review.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

I thought about something.

I need to check about the library declaration in SDC, if there is an impact and if possible to have a component declared library to provide fonts too.

I will try to do that today.

Like in comment #3565258-67: Support library-specific aggregates and next comments, should we handle fonts from component libraries in a follow-up issue?

Follow-up issue created #3586470: Support preloading of fonts declared from components.

Back to RTBC :)

Committed 1c65c7a and pushed to main. Thanks!
Committed ac409b9 and pushed to 11.x. Thanks!

Bah - this was wrong to try to resolve merge conflicts on the fly... ho hum. reverted from 11.x... will open an MR against 11.x

Committed 26b9607 and pushed to 11.x. Thanks!

Follow up issue #3586470: Support preloading of fonts declared from components to add this feature in SDC ready for review.