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

Set and Map support for _.isEmpty #2808

Closed
wants to merge 1 commit into from

Conversation

ushakov-igor
Copy link

Added support for Set and Map for _.isEmpty

@patseev
Copy link

patseev commented May 29, 2019

That's a nice idea. Used to write my own wrapper for this functionality

@jgonggrijp
Copy link
Collaborator

@ushakov-igor Thank you for taking the effort to submit a pull request.

I think this would be nice to have, but it is a breaking change, so we have to be careful. Your PR may remain open for a while because of this.

For consistency, it would be best if all collection functions would treat Map and Set in the same way. This is not necessarily a bad idea, but it does make this a rather big breaking change.

I also recently learned from #2840 that the toString-based type checks like isMap are relatively slow. So to avoid a big slowdown in all functions that support Map and Set, there would have to be an isAssociative fuzzy checker analogous to the internal isArrayLike. I imagine the implementation might go like this:

var getSize = shallowProperty('size');
var getForEach = shallowProperty('forEach');

function isAssociative(collection) {
    var size = getSize(collection);
    var forEach = getForEach(collection);
    return typeof size == 'number' && typeof forEach == 'function' && size >= 0 && size <= MAX_ARRAY_INDEX;
}

@jashkenas
Copy link
Owner

For consistency, it would be best if all collection functions would treat Map and Set in the same way. This is not necessarily a bad idea, but it does make this a rather big breaking change.

Agreed. If we’re going to support Maps and Sets (and WeakMaps and WeakSets?), we should do it across the board, in a single release, and not piecemeal function-by-function.

@jashkenas jashkenas closed this Apr 18, 2020
@jgonggrijp
Copy link
Collaborator

WeakMaps and WeakSets cannot be iterated, so I don't think there's much to support about them in collection functions.

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.

4 participants