- Notifications
You must be signed in to change notification settings - Fork855
Remove lodash and use our own utils#535
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
itsmichaeldiego commentedMar 12, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@ZAKdev Let me know if this change works with your heatmaps. I would really prefer you to make a PR on its own but I can go either way. |
src/google_heatmap.js Outdated
| option => instance.set(option, options[option]), | ||
| Object.keys(options || {}) | ||
| ); | ||
| Object.keys(options).map(option => instance.set(option, options[option])); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I don't know why but i am not able to run entire project, so everything looks good to me just map over an empty object as well if there is no options, like below.
Object.keys(options || {}).map(option => instance.set(option, options[option]));
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@ZAKdev Done, instead of doing what you said I've set a default value for the parameter. LMK How that works. I am merging this as for now.
This is what i am getting from every branch.
|
5419725 to77906a7Compare@itsmichaeldiego, i can't push any of my code, seems like not having access. |
@ZAKdev sorry, push to where? |
@itsmichaeldiego, i added weight in my heapmap implementation but i am not able to push my code to this repo. Do we have any slack channel where we can comminicate or via email, here is my emailzainahmedkhan@gmail.com |
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Uh oh!
There was an error while loading.Please reload this page.
This PR is just one proposal to use specific packages from lodash instead of importing the whole library.
I came across with the problem that we tried to remove lodash here:670fadd#diff-b9cfc7f2cdf78a7f4b91a753d10865a2R72 but we actually moved to devDependencies.
But accidentally, because it was there,@ZAKdev used
fpfrom lodash, as it was still in the devdependencies, in#441.After that, I also used isEmpty from lodash in#533
This increased the build size to 797kb standard and 201kb minified.
This PR decreases it to 140kb and 47.5kb minified.
Output: Size decreased by almost 4 times.
Prev:
Now: