angular-tree-component icon indicating copy to clipboard operation
angular-tree-component copied to clipboard

remove mobx, angular-mobx, lodash, lodash-es dependencies

Open elvisbegovic opened this issue 6 years ago • 11 comments

Can we imagine this awesome lib without these depedances: mobx, angular-mobx, lodash, lodash-es, they are totally increase my build size, tree shaking not works on them ! What people think ?

Cheers

elvisbegovic avatar Feb 23 '18 11:02 elvisbegovic

I really think something should be done about this. I've added #526, but there has been no response. The full lodash library is ~526KB, which is entirely unnecessary given angular-tree-component only uses 15 or so methods from it. Even then, it seems somewhat silly the version is locked down specifically to 4.17.4. If I depend on 4.17.5, I'll have 2 versions of Lodash in my build.

I don't know how the tree-component would be able to get rid of mobX though. It seems so intertwined with everything, but I really don't even know it's purpose.

Regardless, this library and it's dependencies are about 1006 KB, almost a full MB. I would call that extremely bloated when the dist directory itself is only ~280 KB, about 1/4th the size.

mtraynham avatar Feb 27 '18 20:02 mtraynham

@mtraynham i think lib developer is attached to theses dependencies, damage coz imo rxjs cover all needs, i think too the bundle size is big

elvisbegovic avatar Feb 28 '18 07:02 elvisbegovic

Latest mobx version caused karma failures on our system Error: [mobx] Autorun expects a function as first argument [INFO] at invariant (node_modules/mobx/lib/mobx.umd.js:2631:15) [INFO] at Object.autorun (node_modules/mobx/lib/mobx.umd.js:533:9) [INFO] at MobxAutorunDirective.autoDetect (node_modules/mobx-angular/dist/mobx-angular.umd.js:286:29) [INFO] at MobxAutorunDirective.ngOnInit (node_modules/mobx-angular/dist/mobx-angular.umd.js:276:14) [INFO] at checkAndUpdateDirectiveInline (node_modules/@angular/core/bundles/core.umd.js:12407:19) [INFO] at checkAndUpdateNodeInline (node_modules/@angular/core/bundles/core.umd.js:13931:20) [INFO] at checkAndUpdateNode (node_modules/@angular/core/bundles/core.umd.js:13874:16) [INFO] at debugCheckAndUpdateNode (node_modules/@angular/core/bundles/core.umd.js:14767:76) We will be pegging [email protected], but I add my vote to remove dependencies on mobx...

lroitman avatar Mar 12 '18 16:03 lroitman

Hi, Published - can you try with version 7.0.2-beta1?

@mtraynham merged #526 - published in above release @lroitman - should also be fixed in coming publish (I changed mobx version to ^3) @istiti - I'm never attached to libraries, only to people :) MobX is much simpler than RxJS, which is an overkill IMO. MobX is around 60KB in size BTW.

adamkleingit avatar Mar 14 '18 09:03 adamkleingit

@adamkleingit yes but rxjs is already here better use it, and then there is lodash then lodash-es, this is juste my opinion im not criticize, lib's awesome and would be more awesome without dep

thanks

elvisbegovic avatar Mar 14 '18 09:03 elvisbegovic

@adamkleingit thanks for merging #526, reduces my build size by ~372 KB (pics below)!

There does seem to be one more thing that could further reduce the build, which is laxing the dependency range of lodash. This would allow package managers to resolve the same versions more often and reuse modules in the build. Personally, I think it's safe to allow "lodash": "^4".

It might also be ok to lax the mobx and mobx-angular dependencies, even though you just pegged them, to the specific major version (3, not 4, as 4 breaks the component). Really depends on how careful you want to be with minor vs patch updates. Usually you shouldn't see API changes with minor bumps (if they are doing it right).

I'm not sure how many people are using mobx, but rx-js is basically a requirement for Angular 2+, so I'm in agreement with @istiti, it would be nice without mobx.

Before: before After: after

mtraynham avatar Mar 14 '18 15:03 mtraynham

@adamkleingit , thanks a lot, that helped!

lroitman avatar Mar 14 '18 22:03 lroitman

@mtraynham - will relax the versions in the next release. I am a bit skeptic about other people using semver, but I think in this case it should be OK. @istiti - first of all I totally appreciate opinions and always happy to get feedback and suggestions, sorry if it got interpreted otherwise. I just feel very productive with MobX and I think the library is better thanks to it, so I decided to compromise the size in this case. Same with lodash. Any suggestions to reduce size more will be welcome Thanks

adamkleingit avatar Mar 25 '18 21:03 adamkleingit

@adamkleingit LGTM thank you!

mtraynham avatar Mar 26 '18 15:03 mtraynham

@tobiasengelhardt can this issue be reevaluated/open ? (a work was tried here https://github.com/e-cloud/ngx-tree). We should try to avoid deps a max to get angular's update smooth. Thanks for 10.0.0 bdw it wrks on ng10.0.6

elvisbegovic avatar Jul 29 '20 11:07 elvisbegovic

@elvisbegovic thanks for bringing this issue up again. I also thought about it and did not know there was already a discussion about it. angular-mobx is gone now, but mobx and lodash-es are still there and deep imports. We kept it the same for now to not introduce more breaking changes, but this should change.

tobiasengelhardt avatar Jul 30 '20 06:07 tobiasengelhardt