Skip to content

Conversation

ehmicky
Copy link
Contributor

@ehmicky ehmicky commented Mar 25, 2020

Fixes #1048

This removes the omit.js dependency. Besides not being well maintained, it includes an old version of core-js which prints warning messages on npm install.

We do not want to use lodash.omit because it is huge (1500 lines of code compared to 12 lines of code for omit.js). This is because lodash.omit comes with all the extraneous features and utility helpers of the whole Lodash ecosystem. Tiny modules are much simpler to debug and less bug-prone.

I could not find any well maintained alternatives on npm except for filter-obj, which we already use. However it needs to be wrapped with a tiny utility function to make it more convenient.

@ehmicky ehmicky added the type: chore work needed to keep the product and development running smoothly label Mar 25, 2020
@ehmicky ehmicky requested a review from erezrokah March 25, 2020 15:22
@ehmicky ehmicky self-assigned this Mar 25, 2020
@codecov
Copy link

codecov bot commented Mar 25, 2020

Codecov Report

Merging #1050 into master will increase coverage by 0.23%.
The diff coverage is 100.00%.

Impacted Files Coverage Δ
packages/build/src/error/serialize.js 100.00% <100.00%> (ø)
packages/build/src/plugins/env.js 100.00% <100.00%> (ø)
packages/build/src/utils/omit.js 100.00% <100.00%> (ø)
packages/config/src/utils/omit.js 100.00% <100.00%> (ø)
packages/cache-utils/src/hash.js 89.04% <0.00%> (+2.73%) ⬆️
packages/build/src/install/main.js 97.77% <0.00%> (+4.44%) ⬆️
Flag Coverage Δ
#macOS 96.72% <100.00%> (+0.29%) ⬆️
#node_13 97.55% <100.00%> (+0.01%) ⬆️
#node_8_3_0 96.55% <100.00%> (?)
#ubuntu 97.66% <100.00%> (+0.12%) ⬆️

@ehmicky ehmicky merged commit e36de01 into master Mar 25, 2020
@ehmicky ehmicky deleted the chore/remove-omit branch March 25, 2020 16:25
@oles
Copy link

oles commented Mar 25, 2020

You're on a roll, @ehmicky.
I had no idea lodash.omit (and most likely the others in similar format) were so bloated.
That's concerning, and a shame.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: chore work needed to keep the product and development running smoothly

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replace omit.js with lodash.omit

3 participants