Webpacker: [4.0.0-rc.1] Javascript_asset_tag is broken

Created on 15 Dec 2018 · 23Comments · Source: rails/webpacker

I've upgraded from 4.0.0-pre.3 to 4.0.0-rc.1 and now I get a really strange error in my application.slim:

Sprockets::Rails::Helper::AssetNotFound at /
The asset "{\"src\"=>\"/packs/vendor-4699109b154b54e1ef4e.js\",
\"integrity\"=>\"sha256-hkCqb1IPUS4z7o4PyMRG7t3+J0U2MgbmVUty9/hviU0= sha384-
En5fK1fCL66RKx2Ovx8/n6AEwom4R6OgFxxNvKdrL6nQYow56QN66K0CE0N1c2z/ sha512-
nTC02x/4ytkiw9of3MH55Xm4KkZaaGnbEkqBJ+OeToZmQH+fYrasEosB95s+bvNKQ6tjtcEIe
zHJE8aHCyDqSw==\"}.js" is not present in the asset pipeline.

This errors seems to be caused by this call in my project:

_app/views/layout/application.slim_

= javascript_pack_tag 'vendor'

It seems that javascript_pack_tag generates a hash and is incorrectly converted to a Rails include tag. It seems to think that the hash is the src.

Edit: It might have something to do with my splitChunks configuration
This was working fine in 4.0.0-pre.3, but now something goes wrong in manifest.rb - lookup.

environment.config.merge({
  optimization: {
    splitChunks: {
      cacheGroups: {
        commons: {
          test: /\/node_modules\//,
          name: 'vendor',
          chunks: 'all'
        }
      }
    }
  }
});

Configurations with cacheGroups no longer work?

Source

yvbeek

Most helpful comment

It working now. Thanks.

seuros on 15 Dec 2018

👍2

All 23 comments

Made a new release, please give it a try 🙏

gauravtiwari on 15 Dec 2018

👍1

It working now. Thanks.

seuros on 15 Dec 2018

👍2

great, thanks @seuros

gauravtiwari on 15 Dec 2018

Thanks @gauravtiwari !

Unfortunately there is something not right yet.

I use the following packs:

platform.ts: for things like Rollbar that I want to be loaded early on in my page
vendor.ts: for the third party libraries, basically anything from node_modules
tenant.ts: for the actual app logic

I include platform.ts near the top and a bit further down tenant.ts in my slim file:

= javascript_pack_tag 'platform'
...    
= javascript_pack_tag 'tenant', defer: true

In version 4.0.0-pre.3 I had to include vendor manually with a javascript_pack_tag, but version 4.0.0-rc.* seems to takes care of this automatically.

The problem is that in the final HTML the vendor script file is included multiple times:

<script src="/packs/vendor-c74fedd1b41bbb546647.chunk.js"></script>
<script src="/packs/platform-75df7bfe6d1333d19e71.js"></script>
...
<script src="/packs/vendor-c74fedd1b41bbb546647.chunk.js" defer="defer"></script>
<script src="/packs/tenant-218fa49335d9af3f1b19.js" defer="defer"></script>

It probably detects the dependency of both platform and tenant on the vendor pack.

yvbeek on 16 Dec 2018

@Zyphrax Thanks, I noticed the problem. I didn't realize this might have implications on cacheGroups but it seems it's being bundled together with the entry points, which sort of makes sense since it's a dependency like you said.

The idea with this was to include in runtime chunks so you don't have to especially include them or add an option, which one might easily forget. Perhaps, we only check for runtime chunks and include them and leave the rest out so you can include them manually?

gauravtiwari on 16 Dec 2018

Could a simple solution here be to store the list of non-entry point javascript files that have been loaded, and skip adding the <script> if it is already done?

If the case above, this would add the vendor pack once (before platform), and skip it when including tenant.

renchap on 16 Dec 2018

@renchap Yep, it's possible but I think it's best to exclude anything that's not related to a pack and one can include that manually, for ex: vendor can be included anywhere, under the <head />or <body>.

Wrote a fix, will make a PR in a bit.

gauravtiwari on 16 Dec 2018

I am going to take a look at this properly tomorrow if anyone has ideas feel free to make a PR.

gauravtiwari on 16 Dec 2018

I am not sure why you do not want to do it automatically, this could make it much easier for people. Or maybe with a setting to disable this behaviour?

renchap on 16 Dec 2018

Mainly, just to make things easier if it's a runtime chunk. It's quite easy to forget one if people are using splitChunks and forget to include runtime chunk above the main chunk. That's why felt it might be better to include it but didn't realize this might have implications.

gauravtiwari on 16 Dec 2018

I agree that it is a good idea to automatically include dependent packs, and my proposal is to only add something so a dependent pack only get included once per page. With an optional setting to disable this automatic behaviour if people want to do it manually for whatever reason.

renchap on 16 Dec 2018

I also think that if a stylesheet exists for a pack it should be included automatically in the HTML as well, but this may be more complex (as stylesheets should be in <head> where scripts are often at the end of <body>, so you cant just add the <link> tag above/under the related <script> one. This is another recurring topic in this repo's issues :)

renchap on 16 Dec 2018

I really like the functionality that (dependency) packs are automatically included in the page. This indeed makes it a lot easier to handle the runtime chunk, but also the scenario of splitChunks without a name (resulting in packs named 0, 1, 2, etc.).

Maybe calls to javascript_pack_tag and stylesheet_pack_tag could keep track of which scripts / stylesheets are already inserted in the page. In my example the platform pack relies on the vendor pack, so it makes sense to include it before the platform pack in the page.

The javascript_pack_tag and stylesheet_pack_tag could accept a includeDependencies (or shorter alternatives like includeDeps) option to turn off the automatic inclusion of dependent packs.

That would make the following scenarios possible:

Fully automatic

= javascript_pack_tag 'platform'    (vendor is automatically included)
...
= javascript_pack_tag 'tenant'

Semi automatic

= javascript_pack_tag 'vendor'
...
= javascript_pack_tag 'platform'    (ignores vendor, already included)
...
= javascript_pack_tag 'tenant'      (ignores vendor, already included)

Manual

= javascript_pack_tag 'platform', includeDependencies: false
...
= javascript_pack_tag 'vendor'
= javascript_pack_tag 'tenant'      (ignores vendor, already included)

I would keep stylesheets separate from the javascripts. A pack(s)_tag could be introduced to make the inclusion of both javascripts and stylesheets fully automatic, but it would be confusing to include stylesheets with a javascript_pack_tag.

yvbeek on 17 Dec 2018

Thanks for your input @Zyphrax and @renchap, some great ideas there.

I will put together a PR later this week. If someone wants to work on this please leave a comment here. Hopefully, we will have webpacker 4 stable released just around Christmas time 🎉 🎅

Thanks, everyone for being patient and helpful 🙏

gauravtiwari on 17 Dec 2018

I would +1 on what @Zyphrax said, automatic dependency inclusion could avoid a lot of headaches, knowing that webpack changes extracted chunks name often (if not overridden) and have different settings for splitChunks for development and production. The later caused for us broken pages sometimes because of missing files in prod while being totally present in dev. (4.0.0-pre + splitChunks activated)

somebody32 on 17 Dec 2018

RE: https://github.com/rails/webpacker/issues/1835#issuecomment-447615232, @Zyphrax You might try renaming commons to vendors and removing the name prop. I'm not sure if that would cause a problem, but name seems to be in the wrong spot. My bad for not reading closer.

@gauravtiwari Ideally, with the way webpack-assets-manifest generates the entrypoints: { myChunkName: { js: [...], css: [...] }} arrays, _should_ webpacker map all the ordered filenames to <script>s & styles? If so, that would align well with how vanilla webpack is used -- plus there are some nifty features that splitChunks brings to the table.

jakeNiemiec on 18 Dec 2018

@jakeNiemiec My configuration is the same as the second example on Webpack’s splitChunks page. It gobbles up all vendor libraries into one big pack, which is fine for my project.

@gauravtiwari Thank you for all the great work!

yvbeek on 18 Dec 2018

👍1

Temporary workaround for people that have duplicate includes:

/ Don't include dependencies, write only the platform script tag
= javascript_include_tag asset_pack_path('platform.js')

/ Include dependencies, write script tags for vendor and tenant
= javascript_pack_tag 'tenant'

yvbeek on 19 Dec 2018

Please give new RC a try.

gauravtiwari on 17 Jan 2019

@Zyphrax duplicate includes shouldn't be a problem right? The browser has the content cached already?

Do you think that chunks with side-effects (import 'sideEffect.js) will be affected by only outputting unique <script> tags? I worry about older libraries or frameworks that need to be loaded in exactly the right way. (like Angular ngZone)

jakeNiemiec on 17 Jan 2019

@jakeNiemiec Duplicate includes causes the JavaScript code to be executed twice (or as many times as the script is included). It is a waste of performance and it could indeed have side-effects on older libraries that depend on code being loaded in a specific order or utility frameworks like error handlers.

@gauravtiwari Thank you! I'm going to give it a spin!

yvbeek on 22 Jan 2019

👀1

@gauravtiwari Works great, thank you! 🎉

yvbeek on 22 Jan 2019

Duplicate includes causes the JavaScript code to be executed twice (or as many times as the script is included). It is a waste of performance and it could indeed have side-effects on older libraries that depend on code being loaded in a specific order or utility frameworks like error handlers.

My point is that you should always treat vendor libs (especially older libs) as "effectful imports".

Splitting your app into multiple packs, just to load them on the same page is the waste of performance and compile time. Webpack is assuming that those different entrypoints are on exclusive pages (hence the duplication).

take: <%= javascript_packs_with_chunks_tag 'calendar', 'map' %>

This would be the proper way to make the entrypoint:

// calendarMapPack.js
import 'calendar';
import 'map';

// OR async in parallel
import(/* webpackMode: "lazy" */
       /* webpackChunkName: "calendar" */
       'calendar');
import(/* webpackMode: "lazy" */
       /* webpackChunkName: "map" */
       'map');

Any sub-files that you need to load would be within the single entrypoint. With the way it is now, you are fighting and negating any tree-shaking. Plus, you will start getting issues with a lot of weird edge cases (already happening). Take a look at some examples of this: https://github.com/webpack/webpack/issues/6571#issuecomment-426341645

jakeNiemiec on 22 Jan 2019

Was this page helpful?

0 / 5 - 0 ratings