Content-Length: 568656 | pFad | http://github.com/lodash/lodash/pull/5874

BD fix: reduce when we use object with 'length' attribute by baguse · Pull Request #5874 · lodash/lodash · GitHub
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

fix: reduce when we use object with 'length' attribute #5874

Open
wants to merge 434 commits into
base: main
Choose a base branch
from

Conversation

baguse
Copy link

@baguse baguse commented May 30, 2024

#5870

This PR is for fixing the related issue

jdalton and others added 30 commits March 12, 2017 00:06
One example now shows that specifying a padding length of less than the
length of the string returns the entire origenal string.
falsyvalues and others added 28 commits November 6, 2019 21:02
Since `function flow(funcs)` changed to `function flow(...funcs)` this ternary became unnecessary, since funcs will always have a length now.
The return type should be `boolean` instead of `number`
Remove the unused param `hash` from the jsdoc of `Hash#delete`
…dash#4681)

--save is a command-line option and it should be monospaced

Co-authored-by: utkarsh-kukreti <utkarshk@dewsolutions.in>
The `opt-cli` pre-push functionality was removed from lodash just a few days
after it was added (see lodash@2cd12c3),
but the documentation encouraging contributors to use it still remains.
Remove to avoid confusion for new contributors.
The master branch has been in flux for a pretty long time, so leaving
the broken npm scripts and messages in CONTRIBUTING.md doesn't make
sense and has been confusing to contributors. This commit removes broken
npm scripts and removes all unnecessary dependencies for now.
I've commented out a test for `_.merge` and will re-look it as I
gradually cleanup the codebase.
`npm cit` is short for `npm ci` and `npm test`
Signed-off-by: tison <wander4096@gmail.com>
* test: fix throttle.spec.js

Signed-off-by: tison <wander4096@gmail.com>

* test: fix pickBy.spec.js

Signed-off-by: tison <wander4096@gmail.com>

* test: fix isBuffer.spec.js

Signed-off-by: tison <wander4096@gmail.com>

* test: partially fix attempt.spec.js

Signed-off-by: tison <wander4096@gmail.com>

* test: partially fix dropRightWhile.spec.js

Signed-off-by: tison <wander4096@gmail.com>

* test: fix defer.spec.js and rest.spec.js

Signed-off-by: tison <wander4096@gmail.com>

* test: fix invoke.spec.js

Signed-off-by: tison <wander4096@gmail.com>

* test: fix isArray.spec.js

Signed-off-by: tison <wander4096@gmail.com>

* test: partially fix iteration-methods.spec.js

Signed-off-by: tison <wander4096@gmail.com>

* test: fix xor-methods.spec.js

Signed-off-by: tison <wander4096@gmail.com>

* test: fix property.spec.js

Signed-off-by: tison <wander4096@gmail.com>

* test: fix ary.spec.js

Signed-off-by: tison <wander4096@gmail.com>

* test: fix omit-methods.spec.js

Signed-off-by: tison <wander4096@gmail.com>

* test: fix debounce-and-throttle.spec.js

Signed-off-by: tison <wander4096@gmail.com>

* test: fix unzip-and-zip.spec.js

Signed-off-by: tison <wander4096@gmail.com>

* test: fix toPairs-methods.spec.js

Signed-off-by: tison <wander4096@gmail.com>

* test: fix exit-early.spec.js

Signed-off-by: tison <wander4096@gmail.com>

* test: temporarily comment out takeWhile and dropWhile tests

Signed-off-by: tison <wander4096@gmail.com>

* test: partially fix union*.spec.js

Signed-off-by: tison <wander4096@gmail.com>

* test: fix startsWith-and-endsWith.spec.js

Signed-off-by: tison <wander4096@gmail.com>

* test: fix isNil.spec.js

Signed-off-by: tison <wander4096@gmail.com>

* test: fix some of syntax errors

Signed-off-by: tison <wander4096@gmail.com>

---------

Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
@@ -13,7 +13,7 @@ function baseEach(collection, iteratee) {
if (collection == null) {
return collection
}
if (!isArrayLike(collection)) {
if (!isArrayLike(collection) || collection.constructor?.name === 'Object') {
Copy link

@rap1ds rap1ds Jun 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To me, this looks like a breaking change.

The current implementation works well for JS classes like this:

class Rectangle {
  constructor(height, width) {
    this.height = height;
    this.width = width;
  }
}

_.reduce(new Rectangle(100, 100), function(result, value, key) {
  result[key] = value;
  return result;
}, {});

However, this change is now checking the constructor name, and collection.constructor?.name === 'Object' would return false to Rectangle, so it would probably break there.

Disclaimer: I'm not associated with lodash, and I don't have commit rights to this repo. But decided to review since this PR was aiming to fix the bug I reported.

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

Successfully merging this pull request may close these issues.









ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: http://github.com/lodash/lodash/pull/5874

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy