-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
fix: Better support for imageSrcSet
+ imageSizes
in route links
(#184)
#2944
Conversation
imageSrcSet
+ imageSizes
in route links
(#184)
I’ve found these kinds of tests to be flakey because the browser can decide not to prefetch given different memory/cpu/who-knows-what. Maybe we can assert on the rendered link tags instead of requests? |
Also, maybe we can just use the DOM types here? |
Unfortunately the DOM types also require |
🤖 Hello there, We just published version Thanks! |
🤖 Hello there, We just published version Thanks! |
…s` (remix-run#184) (remix-run#2944) Improves support for returned links that include an `imagesrcset` attribute to preload images when matched against a responsive set of URLs. This only works when `rel === 'preload'` and `as === 'image'`. https://html.spec.whatwg.org/commit-snapshots/cb4f5ff75de5f4cbd7013c4abad02f21c77d4d1c/#attr-link-imagesrcset Prior to this change, a few problems with our implementation: 1. Our types for `HtmlLinkDescriptor` make `href` required. This is true _unless_ a link includes `imagesrcset` and `imagesizes`, though `imagesrcset` and `href` will also work with or without `imagesizes` according to the spec. This change uses an intersection type to account for these cases. 2. React 17 was released prior to these attributes being accepted into the spec and implemented by browsers, so passing the props as camel-cased will issue a warning about unsupported DOM props. Our types recognized this and supported the all-lowercase version to skip the warning and forward the attribute to the node, but React 18 now recognizes the camel-cased version. This change updates our types to use the camel-cased prop names, but it will support either at runtime and normalize the object based on the detected version of React. 3. Because we use `href` + `rel` as a key, links without an `href` will have duplicate keys. So `imageSrcSet` is added as a fallback to prevent that.
🤖 Hello there, We just published version Thanks! |
This PR improves support for returned links that include an
imagesrcset
attribute to preload images when matched against a responsive set of URLs. This only works whenrel === 'preload'
andas === 'image'
. https://html.spec.whatwg.org/commit-snapshots/cb4f5ff75de5f4cbd7013c4abad02f21c77d4d1c/#attr-link-imagesrcsetPrior to this PR, a few problems with our implementation:
HtmlLinkDescriptor
makehref
required. This is true unless a link includesimagesrcset
andimagesizes
, thoughimagesrcset
andhref
will also work with or withoutimagesizes
according to the spec. This PR uses an intersection type to account for these cases.href
+rel
as a key, links without an href will have duplicate keys. SoimageSrcSet
is added as a fallback to prevent that.Closes: #184