[4.0.0-rc.1] Javascript_asset_tag is broken #1835

yvbeek · 2018-12-15T00:12:48Z

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?

The text was updated successfully, but these errors were encountered:

gauravtiwari · 2018-12-15T13:03:00Z

Made a new release, please give it a try 🙏

seuros · 2018-12-15T13:20:55Z

It working now. Thanks.

gauravtiwari · 2018-12-15T13:34:33Z

great, thanks @seuros

yvbeek · 2018-12-16T03:35:47Z

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.

gauravtiwari · 2018-12-16T11:09:32Z

@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?

renchap · 2018-12-16T16:43:14Z

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.

gauravtiwari · 2018-12-16T17:38:15Z

@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 · 2018-12-16T21:58:29Z

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

renchap · 2018-12-16T22:07:47Z

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?

gauravtiwari · 2018-12-16T22:12:29Z

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.

renchap · 2018-12-16T22:19:18Z

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 · 2018-12-16T22:21:10Z

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 :)

yvbeek · 2018-12-16T23:23:05Z

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.

gauravtiwari · 2018-12-17T08:24:26Z

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 🙏

somebody32 · 2018-12-17T11:05:32Z

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)

jakeNiemiec · 2018-12-18T02:59:40Z

~~RE: #1835 (comment), @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.

yvbeek · 2018-12-18T04:39:57Z

@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 · 2018-12-19T01:33:50Z

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'

gauravtiwari · 2019-01-17T22:02:57Z

Please give new RC a try.

jakeNiemiec · 2019-01-17T22:21:11Z

@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)

yvbeek · 2019-01-22T02:09:43Z

@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 · 2019-01-22T02:33:46Z

@gauravtiwari Works great, thank you! 🎉

jakeNiemiec · 2019-01-22T21:26:46Z

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: webpack/webpack#6571 (comment)

gauravtiwari closed this as completed in 2110058 Dec 15, 2018

gauravtiwari mentioned this issue Dec 15, 2018

Stable webpacker 4.0 release #1823

Merged

5 tasks

gauravtiwari reopened this Dec 16, 2018

gauravtiwari mentioned this issue Jan 17, 2019

Add a new javascript_packs_with_chunks_tag tag helper #1898

Merged

gauravtiwari closed this as completed in #1898 Jan 17, 2019

jakeNiemiec mentioned this issue Feb 14, 2019

Allow to filter chunks when using javascript_packs_with_chunks #1944

Closed

jakeNiemiec mentioned this issue Apr 1, 2019

Global javascripts #2039

Closed

jakeNiemiec mentioned this issue May 20, 2019

MissingEntryError with javascript_packs_with_chunks_tag but not with javascript_pack_tag #1967

Closed

jakeNiemiec mentioned this issue Aug 5, 2019

multiple websockets being opened when using HMR #2209

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[4.0.0-rc.1] Javascript_asset_tag is broken #1835

[4.0.0-rc.1] Javascript_asset_tag is broken #1835

yvbeek commented Dec 15, 2018 •

edited

Loading

gauravtiwari commented Dec 15, 2018

seuros commented Dec 15, 2018

gauravtiwari commented Dec 15, 2018

yvbeek commented Dec 16, 2018

gauravtiwari commented Dec 16, 2018

renchap commented Dec 16, 2018

gauravtiwari commented Dec 16, 2018

gauravtiwari commented Dec 16, 2018

renchap commented Dec 16, 2018

gauravtiwari commented Dec 16, 2018

renchap commented Dec 16, 2018

renchap commented Dec 16, 2018 •

edited

Loading

yvbeek commented Dec 16, 2018 •

edited

Loading

gauravtiwari commented Dec 17, 2018

somebody32 commented Dec 17, 2018

jakeNiemiec commented Dec 18, 2018 •

edited

Loading

yvbeek commented Dec 18, 2018

yvbeek commented Dec 19, 2018 •

edited

Loading

gauravtiwari commented Jan 17, 2019

jakeNiemiec commented Jan 17, 2019

yvbeek commented Jan 22, 2019

yvbeek commented Jan 22, 2019

jakeNiemiec commented Jan 22, 2019

[4.0.0-rc.1] Javascript_asset_tag is broken #1835

[4.0.0-rc.1] Javascript_asset_tag is broken #1835

Comments

yvbeek commented Dec 15, 2018 • edited Loading

gauravtiwari commented Dec 15, 2018

seuros commented Dec 15, 2018

gauravtiwari commented Dec 15, 2018

yvbeek commented Dec 16, 2018

gauravtiwari commented Dec 16, 2018

renchap commented Dec 16, 2018

gauravtiwari commented Dec 16, 2018

gauravtiwari commented Dec 16, 2018

renchap commented Dec 16, 2018

gauravtiwari commented Dec 16, 2018

renchap commented Dec 16, 2018

renchap commented Dec 16, 2018 • edited Loading

yvbeek commented Dec 16, 2018 • edited Loading

gauravtiwari commented Dec 17, 2018

somebody32 commented Dec 17, 2018

jakeNiemiec commented Dec 18, 2018 • edited Loading

yvbeek commented Dec 18, 2018

yvbeek commented Dec 19, 2018 • edited Loading

gauravtiwari commented Jan 17, 2019

jakeNiemiec commented Jan 17, 2019

yvbeek commented Jan 22, 2019

yvbeek commented Jan 22, 2019

jakeNiemiec commented Jan 22, 2019

yvbeek commented Dec 15, 2018 •

edited

Loading

renchap commented Dec 16, 2018 •

edited

Loading

yvbeek commented Dec 16, 2018 •

edited

Loading

jakeNiemiec commented Dec 18, 2018 •

edited

Loading

yvbeek commented Dec 19, 2018 •

edited

Loading