Skip to content
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

Add preload_link_tag helper. #31251

Merged
merged 3 commits into from Nov 29, 2017
Merged

Add preload_link_tag helper. #31251

merged 3 commits into from Nov 29, 2017

Conversation

@guilleiguaran
Copy link
Member

@guilleiguaran guilleiguaran commented Nov 28, 2017

Summary

This helper creates a link tag with preload keyword that allows to the browser to initiate early fetch of resources (different to the specified in javascript_include_tag and stylesheet_link_tag). Additionally, this sends Early Hints if supported.

See 59a02fb for more details about Early Hints.

Preload spec: https://w3c.github.io/preload/

Usage examples

preload_link_tag("custom_theme.css")
# => <link rel="preload" href="/assets/custom_theme.css" as="style" type="text/css" />

preload_link_tag("/videos/video.webm")
# => <link rel="preload" href="/videos/video.mp4" as="video" type="video/webm" />

preload_link_tag(post_path(format: :json), as: "fetch")
# => <link rel="preload" href="/posts.json" as="fetch" type="application/json" />

preload_link_tag("worker.js", as: "worker")
# => <link rel="preload" href="/assets/worker.js" as="worker" type="text/javascript" />

preload_link_tag("//example.com/font.woff2")
# => <link rel="preload" href="//example.com/font.woff2" as="font" type="font/woff2" crossorigin="anonymous"/>

preload_link_tag("//example.com/font.woff2", crossorigin: "use-credentials")
# => <link rel="preload" href="//example.com/font.woff2" as="font" type="font/woff2" crossorigin="use-credentials" />

preload_link_tag("/media/audio.ogg", nopush: true)
# => <link rel="preload" href="/media/audio.ogg" as="audio" type="audio/ogg" />
@guilleiguaran guilleiguaran force-pushed the preload_link_tag branch to 3cebecb Nov 28, 2017
Copy link
Member

@eileencodes eileencodes left a comment

Looks good to me @guilleiguaran 👍


Mime::Type.register "image/png", :png, [], %w(png)
Mime::Type.register "image/jpeg", :jpeg, [], %w(jpg jpeg jpe pjpeg)
Mime::Type.register "image/gif", :gif, [], %w(gif)
Mime::Type.register "image/bmp", :bmp, [], %w(bmp)
Mime::Type.register "image/tiff", :tiff, [], %w(tif tiff)
Mime::Type.register "image/svg+xml", :svg
Mime::Type.register "image/webp", :webp, [], %w(webp)

Mime::Type.register "audio/mpeg", :mpeg, [], %w(mp1 mp2 mp3)

This comment has been minimized.

@y-yagi

y-yagi Nov 28, 2017
Member

I'm curious. Is it safe for the same value to be registered in mine type symbols? This seems to cause warning in the definition of mime method.
https://travis-ci.org/rails/rails/jobs/308298401#L1970

This comment has been minimized.

@guilleiguaran

guilleiguaran Nov 29, 2017
Author Member

That's right!! I'll remove the video/mpeg one since it contains old formats.

@matthewd
Copy link
Member

@matthewd matthewd commented Nov 29, 2017

Are these really common enough as HTTP responses to be in our default set? 😕

This helper creates a link tag with preload keyword that allows to
browser to initiate early fetch of resources. Additionally this send
Early Hints if supported.

See 59a02fb
for more details about Early Hints.

Preload spec: https://w3c.github.io/preload/
@guilleiguaran guilleiguaran force-pushed the preload_link_tag branch from 3cebecb to 729a3da Nov 29, 2017
Mime::Type.register "audio/flac", :flac, [], %w(flac)

Mime::Type.register "video/webm", :webm, [], %w(webm)
Mime::Type.register "video/mp4", :mp4, [], %w(mp4 m4v)

This comment has been minimized.

@y-yagi

y-yagi Nov 29, 2017
Member

It seems that mp4 is still duplicated, is this intentional?
https://travis-ci.org/rails/rails/jobs/308761133#L1969

This comment has been minimized.

@guilleiguaran

guilleiguaran Nov 29, 2017
Author Member

Missed this comment before of merge, will fix it now :/

@guilleiguaran guilleiguaran merged commit 17661b8 into master Nov 29, 2017
3 checks passed
3 checks passed
codeclimate All good!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@guilleiguaran guilleiguaran deleted the preload_link_tag branch Nov 29, 2017
@guilleiguaran
Copy link
Member Author

@guilleiguaran guilleiguaran commented Nov 29, 2017

@matthewd sorry, missed your comment before of merging, probably those types aren't common enough in HTTP responses but I decided to include those in this set because it seems to be the unique way we can lookup the mime types using only the extension.

I was planning to leave the type attribute out of the tag by default but it seems to be important for browsers:

<link> elements can accept a type attribute, which contains the MIME type of the resource the element is pointing to. This is especially useful when preloading resources — the browser will use the type attribute value to work out whether it supports that resource, and will only start downloading it if this is the case, ignoring it if not.

@matthewd
Copy link
Member

@matthewd matthewd commented Nov 29, 2017

My concern about moving from a list of very common files to all "reasonable" files, is that it opens us up to a lot of future churn.

And on a very relevant note: it also pretty much obliges us to maintain the values forever. You've removed an existing definition, which will break people's existing applications without a deprecation. We can't do that.

I haven't even heard of half of these things, so I'd rather not be stuck keeping them in a list forever, just to support a small number of users (both because of a rare filetype, and a currently rare feature).

How about we trim the list for now, and just make the helper raise if we don't know the type? That way people will be forced to either specify it as an option, or register the type for themselves.

guilleiguaran added a commit that referenced this pull request Nov 29, 2017
See discussion in #31251
@guilleiguaran
Copy link
Member Author

@guilleiguaran guilleiguaran commented Nov 29, 2017

@matthewd agree, I've restored mpeg and trimmed the list keeping only the most common types in 0061e0c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.