Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use ramda insead of lodash #230

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Use ramda insead of lodash #230

wants to merge 1 commit into from

Conversation

kylef
Copy link
Member

@kylef kylef commented Jun 20, 2019

The biggest motivation was that we're using Ramda elsewhere which provides similiar functionality. This has a decent improvement in the overall bundle size, saving us 96K or 19% of the entire bundle size:

$ du -h dist/*.js
508K    dist/minim-with-lodash.js
412K    dist/minim-with-ramda.js

This has a decent improvement in the overall bundle size, saving us 96K
or 19% of the entire bundle size:

    $ du -h dist/*.js
    508K    dist/minim-with-lodash.js
    412K    dist/minim-with-ramda.js
@kylef kylef requested a review from honzajavorek June 20, 2019 18:38
@kylef
Copy link
Member Author

kylef commented Jun 20, 2019

Added "do not merge", slighly concerned about performance difference for some of the change code paths, doing further analysis.

honzajavorek
honzajavorek previously approved these changes Jun 21, 2019
@kylef
Copy link
Member Author

kylef commented Jun 25, 2019

element.id.equals is now around a 4x slower operation on this branch.

@kylef
Copy link
Member Author

kylef commented Jun 25, 2019

Another problem, clone from Ramda is not a deep clone (ramda/ramda#2850). This behaviour would be breaking minim's interface and likely some of the OAS 2 and OAS 3 parsers.

@honzajavorek honzajavorek dismissed their stale review June 25, 2019 10:22

New facts emerged!

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

Successfully merging this pull request may close these issues.

2 participants