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

Lodash optimization #120

Merged
merged 1 commit into from
Jul 31, 2021
Merged

Lodash optimization #120

merged 1 commit into from
Jul 31, 2021

Conversation

VadimKorobka
Copy link
Contributor

No description provided.

@codecov
Copy link

codecov bot commented Aug 15, 2019

Codecov Report

Merging #120 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #120   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           1      1           
  Lines           3      3           
  Branches        1      1           
=====================================
  Hits            3      3
Impacted Files Coverage Δ
src/index.js 100% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b6b9953...c260b1a. Read the comment docs.

@VadimKorobka
Copy link
Contributor Author

Babel-lodash plugin is not working expected. Please, use lodash modules. I tested it in my project by using webpack-bundle-analyzer.

@heartAndRain
Copy link

@JaKXz Can this PR be merged?

@MarcLopezAvila
Copy link

Please merge this PR, our bundle size is being affected.

@VadimKorobka
Copy link
Contributor Author

VadimKorobka commented Nov 15, 2019

Dear @JaKXz,

To make a decision about PR, I propose to consider two aspects:

  1. It is clear to everyone that these changes will not break anything.
  2. This will really reduce the size of the bundle.

Hence, there is no reason to close PR. Even if you think that tree shaking works, then this change simply will not change anything. BUT if it is not true, then we will get a reduced bundle for many projects using this library.We can merge this PR, and make a beta version, which everyone who needs it will try to switch to it, and a little later release a stable.

I am looking forward for your reply.
Faithfully yours, Vadim

@andreieftimie
Copy link

Can we have this or something similar merged in?
This is pulling a complete lodash copy into our bundle.

@VadimKorobka
Copy link
Contributor Author

VadimKorobka commented Jun 21, 2020

Dear @JaKXz & @timche
We are looking forward to your reply. If it's needed, I can answer the questions.

@VadimKorobka
Copy link
Contributor Author

Dear @JaKXz & @timche
We are looking forward to your reply. If it's needed, I can answer the questions.

@timche timche merged commit f26ab4f into redux-utilities:master Jul 31, 2021
@timche
Copy link
Member

timche commented Jul 31, 2021

Thanks for your PR @VadimKorobka! Sorry that it took so long. I'm not actively maintaining this project anymore. I've created a new release (https://github.com/redux-utilities/flux-standard-action/releases/tag/v2.1.2) and published it on npm (https://www.npmjs.com/package/flux-standard-action).

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

Successfully merging this pull request may close these issues.

7 participants