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 #1874

Closed
dmix opened this issue Jan 7, 2019 · 7 comments
Closed

Add preload_link_tag helper #1874

dmix opened this issue Jan 7, 2019 · 7 comments

Comments

@dmix
Copy link

dmix commented Jan 7, 2019

Rails recently released support for preloading content:

rails/rails#31251

Documented here: https://api.rubyonrails.org/classes/ActionView/Helpers/AssetTagHelper.html#method-i-preload_link_tag

Which is recommended by Google and Mozilla to speed up pageloading performance:

https://developers.google.com/web/fundamentals/performance/resource-prioritization

https://developer.mozilla.org/en-US/docs/Web/HTML/Preloading_content

Any chance we could get similar support for "stylesheet_pack_tag" et al?

@PikachuEXE
Copy link

My own versions of helpers for this: (for 4.0.0.rc3)

def javascript_entrypoint_pack_preload_tag(*names, **options)
  actual_paths = javascript_entrypoint_pack_paths(*names)

  # rubocop:disable Rails/OutputSafety
  actual_paths.uniq.map do |actual_path|
    href = actual_path

    tag_options = {
      rel: "preload",
      href: href,
      as: "script",
    }.reverse_merge(options)

    tag(
      :link,
      tag_options,
    )
  end.join("\n").html_safe
  # rubocop:enable Rails/OutputSafety
end

def javascript_entrypoint_pack_paths(*names)
  logical_paths = sources_from_manifest_entrypoints(names, type: :javascript)

  logical_paths.uniq.map do |logical_path|
    path_to_javascript(logical_path, skip_pipeline: true)
  end
end

@guillaumebriday
Copy link
Member

I did it, I will create a PR in the afternoon 👍

@jakeNiemiec
Copy link
Member

I left feedback about this over here: #2124 (review)

Apart from that, I feel like there is some de-sync happening between all of us. Let me try and straighten it out:


Rails recently released support for preloading content: rails/rails#31251

This is useful for preloading fonts, videos, and web workers, but preloading all of your JS scripts is antithetical to progressive web enhancement and chunk lazy loading. Plus, webpack gives you this as a feature.

is recommended by Google and Mozilla to speed up pageloading performance

In the google link:

Note that is a compulsory instruction to the browser; unlike the other resource hints we’ll be talking about, it’s something the browser must do, rather than merely an optional hint. This makes it particularly important to test carefully, to insure that you’re not accidentally causing anything to fetch twice by using it, or fetching something that’s not needed.

Mozilla does not currently support this at all:

image

Any chance we could get similar support for "stylesheet_pack_tag" et al?

This is my main source of confusion. You don't want to preload the css, you want to preload the fonts and images inside the css so that fonts, css, and images can all be downloaded in parallel instead of waiting for the browser to parse the css.

My own versions of helpers for this...

The initial question was about stylesheet assets. While your approach is completely valid, I am not sure it makes for a good general recommendation to preload all chunks like this.

I would instead let webpack inject the <link rel="preload"> for you with import(/* webpackPreload: true */ "./my/file.js"). You can read more about this here (article linked from docs): https://medium.com/webpack/link-rel-prefetch-preload-in-webpack-51a52358f84c

Webpack docs on preloading: https://webpack.js.org/guides/code-splitting/#prefetchingpreloading-modules

@sharang-d
Copy link
Contributor

Good to close?

@guillaumebriday
Copy link
Member

Looks good to me

@infernalmaster
Copy link

= preload_link_tag(asset_pack_path("application.js"))

@guillaumebriday
Copy link
Member

@infernalmaster What are you trying to achieve ?

You probably don't want/need to preload you application.js file.

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

No branches or pull requests

6 participants