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

6.0.0-beta.5 runtime.js included multiple times with multiple javascript_pack_tag #2932

Closed
daniesg opened this issue Feb 19, 2021 · 10 comments
Closed

Comments

@daniesg
Copy link

daniesg commented Feb 19, 2021

If you have multiple pack tags

<%= javascript_pack_tag "application" %>
<%= javascript_pack_tag "styleguide" %>

One runtime is generated, but the same runtime is included twice.
Screenshot 2021-02-18 at 10 50 23 PM

If you however call javascript_pack_tag with multiple arguments,

<%= javascript_pack_tag "application", "styleguide" %>

The runtime is included only once, like expected.
Screenshot 2021-02-18 at 10 52 32 PM

I understand that webpcker has optimization: { runtimeChunk: 'single' }' to make sure one runtime is generated, but is it possible to make sure runtime is also included in the page only once, even if javascript_pack_tag is called multiple times? Not surprisingly, including it twice results in weird bugs.

@rossta
Copy link
Member

rossta commented Feb 19, 2021

@daniesg Thanks for this report! I agree, I think it would be great if Webpacker could handle multiple pack tags gracefully. I have a few ideas, but need to test them out first.

@daniesg
Copy link
Author

daniesg commented Feb 25, 2021

If there's anything I can try out in a PR, please let me know! I don't know the codebase well at all, but willing to help with coding. I'll take a look at the code now and see if I can come up with something.

Thanks for the quick response btw!

@ferblape
Copy link
Contributor

ferblape commented Mar 9, 2021

BTW, optimization: { runtimeChunk: 'single' }' didn't work to me, but optimization: { runtimeChunk: false }

@rossta
Copy link
Member

rossta commented Mar 26, 2021

@ferblape That solution "sort of" works. The "single" runtime chunk wouldn't be the only shared script—there could be multiple "vendor" chunks that get duplicated as well. There might not be duplication in your case, but this may not be a solution for others.

webpack doesn't expect there to be multiple places where developers want to insert tags as is customary in Rails. I suspect the only way to solve this problem is handle this in Webpacker.

One idea is represented by this imaginary API change to Webpacker below to force script tags to be rendered in only place. I'm not sure if something like this can be built-in to the normal usage and it might not work for all the ways developers might want to insert scripts on a page.

<%= render_pack_tags %>

<%= include_javascript_pack_tag "application" %>

<%= include_javascript_pack_tag "styleguide" %>

Another idea is for Webpacker helpers to be aware of the request and to keep track of which chunks have already been rendered. This might include additional "smarts" in the helper that may be hard to get right, but possible. I see some other improvements with this approach, such as enabling a fix for #2814.

@samtgarson
Copy link

Hello! FWIW this is fatal for our application—we're using React with different packs containing different components for different pages (i.e. a homepage pack, a user profile pack, etc) along with a shared pack, in order to avoid shipping all components to all pages when they're not needed.

At the moment including multiple javascript_pack_tags is adding duplicate runtime and vendor packs (as mentioned above), meaning we get two of most things and this completely breaks certain React functionality (like HMR in dev and hooks in production).

We've resorted to shipping all JS and CSS to all pages for now, but a fix for this feels pretty essential for complex applications with large client-side components.

Is there any desire to work on a solution for this?

@justin808
Copy link
Contributor

@samtgarson, you need to pass the bundles as multiple arguments to the javascript_pack_tag helper. If you get stuck, ping me on the React plus Rails Slack.

@samtgarson
Copy link

Thanks @justin808 that’s exactly what we’re doing (as you’ll see in the PR I left on your repo), but that doesn’t help with conditionally including different packs from different places.

I.e. we had a yield :page_specific_js in our layout, which allowed each view to add asset tags to the layout (e.g. our homepage view could include the homepage assets, the profile view could include the profile assets, etc) which obviously breaks because of this issue.

We could of course build our own conditional system of somehow building that list of packs to include based on which view we’re currently in, but it would be lovely if Webpacker could handle that for us—as Rails devs we’re used to being able to add JavaScript asset tags as often as we like, and this new behaviour is non-obvious and feels like a gotcha (at least to me)

@BuonOmo
Copy link

BuonOmo commented Jul 5, 2021

👍 for this being an important issue!

I wanted to show two hacks we used while no fix exists.

Adding one pack to the main one

<%=
    tags = %w(application)
    tags.push(yield :extra_pack) if content_for?(:extra_pack)
    javascript_pack_tag *tags, async: :async, defer: :defer
%>

Adding many packs

<%=
    tags = %w(application)
    tags.concat((yield :extra_packs).split) if content_for?(:extra_packs)
    javascript_pack_tag *tags, async: :async, defer: :defer
%>

@DRSisco
Copy link

DRSisco commented Aug 3, 2021

Adding onto @BuonOmo's post with some more dynamic magic.

Sometimes in projects where I have a lot of JS in route specific packs I don't want to specify them all out in the views, so I include them implicitly.

Of course, not every route has a pack associated with it and some just used some shared pack instead. If you include packs dynamically or even have a view that includes more packs, then we can check the manifest if the entrypoint exists for that file before sending it to the pack_tag. Additionally, we can include css doing this the same way if the packs also manage the css, otherwise you can skip that.

<%
    tags = %w(application)
    tags.push File.join('controllers', params[:controller], params[:action])
    tags.concat((yield :extra_packs).split) if content_for?(:extra_packs)
	js_tags = tags.filter { |tag| Webpacker.instance.manifest.lookup_pack_with_chunks(tag, type: :javascript) }
	css_tags = tags.filter { |tag| Webpacker.instance.manifest.lookup_pack_with_chunks(tag, type: :stylesheet) }
%>
<%= javascript_pack_tag *js_tags, async: :async, defer: :defer %>
<%= stylesheet_pack_tag *css_tags %>

Hope that helps, for others with a lot of route/controller specific packs.

@dhh
Copy link
Member

dhh commented Aug 25, 2021

Now raising on double calls.

@dhh dhh closed this as completed Aug 25, 2021
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

8 participants