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 a new javascript_packs_with_chunks_tag tag helper #1898

Merged
merged 1 commit into from
Jan 17, 2019
Merged

Conversation

gauravtiwari
Copy link
Member

@gauravtiwari gauravtiwari commented Jan 17, 2019

This PR adds a new chunks helper that addresses issues outlined here: #1835

Fixes #1835

<%= javascript_packs_with_chunks_tag 'hello_react', 'application', 'hello_vue', 'data-turbolinks-track': 'reload' %>

which outputs following html, with depedencies de-deduped

<script src="/packs/runtime~hello_react-26febc8eea663e32aadb.js" data-turbolinks-track="reload"></script>
<script src="/packs/0-d471c80da3d3a8a51c72.chunk.js" data-turbolinks-track="reload"></script>
<script src="/packs/1-8ef1f3ec1d6838a685eb.chunk.js" data-turbolinks-track="reload"></script>
<script src="/packs/hello_react-d26893900192b71057ec.chunk.js" data-turbolinks-track="reload"></script>
<script src="/packs/runtime~application-2a426143d8d4ad5ac92e.js" data-turbolinks-track="reload"></script>
<script src="/packs/application-c45c132bb54ceeafd2e5.chunk.js" data-turbolinks-track="reload"></script>
<script src="/packs/runtime~hello_vue-b068488867759681a7da.js" data-turbolinks-track="reload"></script>
<script src="/packs/2-4bd0417a6b1fb648dbf1.chunk.js" data-turbolinks-track="reload"></script>
<script src="/packs/hello_vue-7e0743cde157093b96bb.chunk.js" data-turbolinks-track="reload"></script>

javascript_packs_with_chunks_tag is introduced to handle new webpack split chunks API: https://webpack.js.org/plugins/split-chunks-plugin/

 "application": {
      "js": [
        "/packs/runtime~application-2a426143d8d4ad5ac92e.js",
        "/packs/0-d471c80da3d3a8a51c72.chunk.js",
        "/packs/application-c45c132bb54ceeafd2e5.chunk.js"
      ],
      "js.map": [
        "/packs/runtime~application-2a426143d8d4ad5ac92e.js.map",
        "/packs/0-d471c80da3d3a8a51c72.chunk.js.map",
        "/packs/application-c45c132bb54ceeafd2e5.chunk.js.map"
      

javascript_pack_tag, will keep working the same way as before, so no breaking changes there.

<%# This will include all deps + pack %>
<%= javascript_packs_with_chunks_tag "stimulus", "hello_stimulus" %>

<%# this will include transpiled packs only, no deps (if using split chunks) %>
<%= javascript_pack_tag "stimulus" %>
<%= javascript_pack_tag "hello_stimulus" %>
<%# DO %>
<%= javascript_packs_with_chunks_tag 'calendar', 'map' %>

<%# DON'T %>
<%= javascript_packs_with_chunks_tag 'calendar' %>
<%= javascript_packs_with_chunks_tag 'map' %>

Thanks @javan, could you please review?

@gauravtiwari gauravtiwari changed the title Add a new javascript_chunks_tag tag helper Add a new javascript_packs_with_chunks_tag tag helper Jan 17, 2019
@gauravtiwari gauravtiwari merged commit c4cf2e9 into master Jan 17, 2019
@gauravtiwari gauravtiwari deleted the pack-tag branch January 17, 2019 21:58
@DougPuchalski
Copy link

Is there an approach to using defer: true? Presently I get different issues popping up when true and false.

@DougPuchalski
Copy link

Also, there's a need to be able to generate a list of all the paths to packs, not just tags, i.e. when building a service worker that caches files.

@gauravtiwari

@jakeNiemiec
Copy link
Member

I'd advise against building a service worker with webpack. You will end up with multiple workers registered under differently hashed names within the same domain. (plus your sw file needs to be in the /public dir to work on some browsers)

@DougPuchalski
Copy link

@jakeNiemiec Not sure what you mean. Are you suggesting that if an app requires service workers then to not use webpack at all? That seems like a huge shortcoming

@jakeNiemiec
Copy link
Member

https://developer.mozilla.org/en-US/docs/Web/API/ServiceWorkerContainer/register#Examples

There is frequent confusion surrounding the meaning and use of scope. Since a service worker can't have a scope broader than its own location, only use the scope option when you need a scope that is narrower than the default.

If your service worker is in www.example.com/packs/SW.js, it can only control pages under www.example.com/packs which does not make much sense.

This is why service workers are generally not "built". Some SW libraries may copy the file to the correct place, it all depends on the implementation.

@DougPuchalski
Copy link

This PR has removed some existing functionality, which was to allow availability of pathnames of packs instead of just tags. This breaks my application unless I use private API's. Opening a discussion to philosophy of service workers is a big diversion and seems out of scope.

Regardless, we now have to choose between using a private API that may go away at any time, turning off code splitting, re-architecting our build process, designing out or reverting to an older version of webpack, etc etc and all of these options mean delays and cost. Is that really how things should go?

@DougPuchalski
Copy link

@gauravtiwari ?

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

Successfully merging this pull request may close these issues.

5 participants