stealjs / steal-tools

Build easy. Load fast.
https://stealjs.com/docs/steal-tools.html
MIT License
67 stars 23 forks source link

stache done-autorender breaks treeshaking #1068

Closed pYr0x closed 5 years ago

pYr0x commented 5 years ago

if you using a doneJS app and you want to create a build you use "main": "stache-treeshaking/index.stache!done-autorender" as main during build.

now treeshaking will not work anymore. functions f() and b() will be in the dest file, but only f() is used.

if you using the view model file like main: "~/app" as main, treeshaking work. only f() is in the dest file

see example app https://github.com/pYr0x/stache-treeshaking see the build file https://github.com/pYr0x/stache-treeshaking/blob/master/build.js#L9 to change to app.js

matthewp commented 5 years ago

If you don't mind trying, what happens if you change this line to if(true) {, does it work? This disables the build-time tree-shaking (rollup) but not the tree-shaking that occurs within steal. I sort of want to remove rollup because it causes many of these issues I think.

pYr0x commented 5 years ago

if i change it to true and run again with stache-treeshaking/index.stache!done-autorender as main i will get the same result. b() is still in dest file. now even if i use app.js b() is present

pYr0x commented 5 years ago

@matthewp any progress on this issue?

matthewp commented 5 years ago

No, sorry for not replying though. I understand the issue now. For some reason it is not seeing that foo is only imported by another ES module. It should tree-shake in that situation, I believe. I'll try and check it out today.

matthewp commented 5 years ago

Breaking test shown here: https://github.com/stealjs/steal-tools/commit/6255900af23188713485246b029b49f7132e69e0

Looking into what is going on.

matthewp commented 5 years ago

PR: https://github.com/stealjs/steal-tools/pull/1070

matthewp commented 5 years ago

2.0.5: https://github.com/stealjs/steal-tools/releases/tag/v2.0.5

pYr0x commented 5 years ago

thank you very much!