feat: remove lodash dependency #743

dpschen · 2021-09-12T16:45:51Z

dpschen commented

2021-09-12 16:45:51 +00:00

This should work on its own but makes more sense to merge after #752 .

Lodash is quite big and all helpers got imported even thought not all needed. With new JS syntax it's ~~easy~~ possible to replace most helpers.

This should work on its own but makes more sense to merge after https://kolaente.dev/vikunja/frontend/pulls/752 . Lodash is quite big and all helpers got imported even thought not all needed. With new JS syntax it's ~~easy~~ possible to replace most helpers.

dpschen force-pushed feature/remove-lodash from 544ee041a3 to 0828cf8eb0

2021-09-12 16:52:29 +00:00

Compare

dpschen force-pushed feature/remove-lodash from 0828cf8eb0 to 2179c0dcca

2021-09-14 11:08:04 +00:00

Compare

dpschen force-pushed feature/remove-lodash from 2179c0dcca to 8399f96c3d

2021-09-14 11:25:00 +00:00

Compare

dpschen force-pushed feature/remove-lodash from 8399f96c3d to 9e3e970d3a

2021-09-14 14:33:07 +00:00

Compare

dpschen commented

2021-09-14 15:34:33 +00:00

Okay, somethink still doesn't work. I can't get cypress to run locally. Will keep this as is for now.

dpschen force-pushed feature/remove-lodash from 9e3e970d3a to 5b8502603b

2021-09-14 15:36:39 +00:00

Compare

konrad commented

2021-09-21 16:55:19 +00:00

Since lodash is tree-shakable and we're only importing what's required, how big is the difference this makes in the final bundle size?

dpschen commented

2021-09-22 19:27:30 +00:00

Since lodash is tree-shakable and we're only importing what's required, how big is the difference this makes in the final bundle size?

It won't make a large difference. I'm not sure lodash was correctly tree shaken before. Either way I think it makes sense to use ES features where they replace lodash 1 to 1. My option in this regard goes in the direction of this article: https://thejs.dev/jmitchell/its-time-to-let-go-of-lodash-nqc

> Since lodash is tree-shakable and we're only importing what's required, how big is the difference this makes in the final bundle size? It won't make a large difference. I'm not sure lodash was correctly tree shaken before. Either way I think it makes sense to use ES features where they replace lodash 1 to 1. My option in this regard goes in the direction of this article: https://thejs.dev/jmitchell/its-time-to-let-go-of-lodash-nqc

dpschen commented

2021-10-02 13:19:47 +00:00

I'm leaving this branch open aswell until vue 3 is finished.

👍 1

dpschen force-pushed feature/remove-lodash from 5b8502603b to db3a502837

2021-10-04 17:19:58 +00:00

Compare

dpschen force-pushed feature/remove-lodash from db3a502837 to 6fb77ad3b7

2021-10-04 17:30:38 +00:00

Compare

dpschen force-pushed feature/remove-lodash from 6fb77ad3b7 to fc267dc644

2021-10-04 20:23:23 +00:00

Compare

dpschen force-pushed feature/remove-lodash from fc267dc644 to 724e776fef

2021-10-04 20:31:47 +00:00

Compare

dpschen force-pushed feature/remove-lodash from 724e776fef to 6c8ec65c6e

2021-10-04 20:40:21 +00:00

Compare

dpschen force-pushed feature/remove-lodash from 6c8ec65c6e to 2d80d08d7a

2021-10-04 21:32:06 +00:00

Compare

dpschen changed title from ~~WIP: feat: remove lodash dependency~~ to feat: remove lodash dependency

2021-10-04 21:36:54 +00:00

dpschen commented

2021-10-04 21:37:32 +00:00

@konrad here we go :)

I hope this can mitigate some problems I currently have with the vue 3 branch.

@konrad here we go :) I hope this can mitigate some problems I currently have with the vue 3 branch.

dpschen force-pushed feature/remove-lodash from 2d80d08d7a to d9f2d879d7

2021-10-04 21:47:44 +00:00

Compare

konrad commented

2021-10-06 20:24:24 +00:00

Interesting to compare the bundle sizes before and after:

Before:

$ yarn build    
yarn run v1.22.10
$ vite build && workbox copyLibraries dist/
vite v2.6.1 building for production...
✓ 1736 modules transformed.

(ommitted some assets)

dist/assets/highlight.b492372e.js           129.07 KiB / gzip: 41.10 KiB
dist/assets/index.19c20266.css              292.00 KiB / gzip: 41.44 KiB
dist/assets/user-settings.f7fda631.js       352.11 KiB / gzip: 104.05 KiB
dist/assets/editor.c1be15a2.js              449.40 KiB / gzip: 149.01 KiB
dist/assets/index.0a54aa68.js               609.87 KiB / gzip: 163.15 KiB

After:

$ yarn build
yarn run v1.22.10
$ vite build && workbox copyLibraries dist/
vite v2.6.1 building for production...
✓ 1361 modules transformed.

(ommitted some assets)

dist/assets/highlight.b492372e.js           129.07 KiB / gzip: 41.10 KiB
dist/assets/index.226fd5b8.css              292.00 KiB / gzip: 41.44 KiB
dist/assets/user-settings.4562e13b.js       331.58 KiB / gzip: 95.62 KiB
dist/assets/editor.f55611fa.js              449.40 KiB / gzip: 149.01 KiB
dist/assets/index.0ee95d8b.js               604.73 KiB / gzip: 160.48 KiB

So while it did save some bytes, the overall difference in bundle size is not that big.

Interesting to compare the bundle sizes before and after: Before: ``` $ yarn build yarn run v1.22.10 $ vite build && workbox copyLibraries dist/ vite v2.6.1 building for production... ✓ 1736 modules transformed. (ommitted some assets) dist/assets/highlight.b492372e.js 129.07 KiB / gzip: 41.10 KiB dist/assets/index.19c20266.css 292.00 KiB / gzip: 41.44 KiB dist/assets/user-settings.f7fda631.js 352.11 KiB / gzip: 104.05 KiB dist/assets/editor.c1be15a2.js 449.40 KiB / gzip: 149.01 KiB dist/assets/index.0a54aa68.js 609.87 KiB / gzip: 163.15 KiB ``` After: ``` $ yarn build yarn run v1.22.10 $ vite build && workbox copyLibraries dist/ vite v2.6.1 building for production... ✓ 1361 modules transformed. (ommitted some assets) dist/assets/highlight.b492372e.js 129.07 KiB / gzip: 41.10 KiB dist/assets/index.226fd5b8.css 292.00 KiB / gzip: 41.44 KiB dist/assets/user-settings.4562e13b.js 331.58 KiB / gzip: 95.62 KiB dist/assets/editor.f55611fa.js 449.40 KiB / gzip: 149.01 KiB dist/assets/index.0ee95d8b.js 604.73 KiB / gzip: 160.48 KiB ``` So while it did save some bytes, the overall difference in bundle size is not that big.

konrad approved these changes 2021-10-06 20:24:42 +00:00

konrad left a comment

Thanks for the effort!

konrad merged commit faa2daa876 into main

2021-10-06 20:25:06 +00:00

konrad deleted branch feature/remove-lodash

2021-10-06 20:25:07 +00:00

konrad referenced this issue from a commit

2021-10-06 20:25:07 +00:00

feat: remove lodash dependency (#743)

dpschen commented

2021-10-07 09:14:57 +00:00

Around 10kb gzipped js is a lot.

Might be that we need to put some of it back though. I might need the deepClone again and the 'JSON.parse' trick doesnt work with e.g. Dates. And the support for structured clone is... well it's complicated :D
https://developer.mozilla.org/en-US/docs/Web/API/structuredClone

Around 10kb gzipped js is a lot. Might be that we need to put some of it back though. I might need the deepClone again and the 'JSON.parse' trick doesnt work with e.g. Dates. And the support for structured clone is... well it's complicated :D https://developer.mozilla.org/en-US/docs/Web/API/structuredClone

konrad commented

2021-10-07 18:19:24 +00:00

Yeah, well... Maybe there's a polyfill for it somewhere? But I guess that would just do what deepClone from loadash is doing already?

dpschen commented

2021-10-07 18:46:49 +00:00

Yes. Just have to doubleCheck (:P) if we really need a deep clone.
I think the lodash/deepClone is still the way to go. But than as single import and install (via lodash.deepclone). The single function packages are deprecated but as long as lodash v5 isn't out I think that's fine.

Yes. Just have to doubleCheck (:P) if we really need a deep clone. I think the lodash/deepClone is still the way to go. But than as single import and install (via `lodash.deepclone`). The single function packages are deprecated but as long as lodash v5 isn't out I think that's fine.

This repo is archived. You cannot comment on pull requests.

No reviewers