marko-js / examples

55 stars 36 forks source link

webpack-express example is serving JS when none is needed #20

Closed vwong closed 3 years ago

vwong commented 3 years ago

Overview

On static pages that don't need JS, some JS is still emitted. It looks to be empty Webpack overheads.

Steps to reproduce

Remove all JS code by removing <app-main /> from src/pages/index/template.marko
Do a production build (to avoid Hot Module Reloading causing false alarms)
Visit the static page

DylanPiercey commented 3 years ago

@vwong I just looked into this one. Looks like this issue is that webpack outputs a little bit of glue code for imported assets (like this https://github.com/marko-js/examples/blob/21c2eede4b9d12cbd4c40981368592c82cb3e7ca/examples/webpack-express/src/components/sample-header/index.marko#L1).

I'd like to find a way to have webpack tree shake that out but I'm not sure what that is.

If you comment out <app-main/> and prevent importing the asset I linked above for the image then you will see there is no JS output.

I'm going to close this since it's more of a webpack issue that anything. If we can find a work around, or way we can handle this specially from the Marko side, I'd be very interested though.

vwong commented 3 years ago

I see you've since simplified the example greatly. That's a shame. I learnt so much from the more complex example.

In any case, it was working fine in https://github.com/marko-js/examples/commit/ddb479bc5b172795068ebd2f99d9d75e6148914d. So, I combed through the changes line by line, and eventually found the one line that broke this behavior. It was https://webpack.js.org/configuration/optimization/#optimizationruntimechunk

Working: optimization.runtimeChunk was undefined, so defaulted to false: each entry chunk embeds runtime

Broken: optimization.runtimeChunk was single: creates a runtime file to be shared for all generated chunks.

Now, I've got no JS if none is needed, even if I reference other assets, like images.

DylanPiercey commented 3 years ago

@vwong thats really good to know. That config is only included in the sample app for a really obscure reason. Essentially it allows you to have multiple top level components passed into the root template and it’d still dedupe assets between them.

I think for this example and really in general this is best removed. Will update the example accordingly. Really appreciate you taking the time to look into it!

vwong commented 3 years ago

No worries. I needed it fixed for my own app anyway!