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

Guard with isIterationCall #2231

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jridgewell
Copy link
Collaborator

Stolen from lodash, used to determine if one of our underscore methods were called by _.each, _.map, or friends.

@jridgewell jridgewell changed the title Add isIterationCall Guard with isIterationCall Jul 9, 2015
@@ -136,15 +136,35 @@
};
};

// Helper to determine if index is within the bounds of an array.
var isIndex = function(index, length) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this helper necessary?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not necessarily, but there's a bit of duplication between isArrayLike and isIterationCall that'll get worse once I propose only considering ints and not floats as proper index.

@megawac megawac mentioned this pull request Jul 9, 2015
6 tasks
@@ -330,7 +353,8 @@
_.min = function(obj, iteratee, context) {
var result = Infinity, lastComputed = Infinity,
value, computed;
if (iteratee == null || (typeof iteratee == 'number' && typeof obj[0] != 'object') && obj != null) {
if (context && isIterationCall(obj, iteratee, context)) iteratee = null;
if (iteratee == null) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Mind adding a test for this change

@jashkenas
Copy link
Owner

Please explain what this is doing, beyond what the previous "guard" flag was already doing?

@jdalton
Copy link
Contributor

jdalton commented Jul 28, 2015

Please explain what this is doing, beyond what the previous "guard" flag was already doing?

It's a more thorough check for when a guard param isn't applicable, like in cases where it's a context param.

@jashkenas
Copy link
Owner

Maybe an example would help clarify? I'm don't see any changed tests in this PR.

megawac referenced this pull request in lodash/lodash Jul 30, 2015
@jgonggrijp
Copy link
Collaborator

For future reference, isIterationCall is a throw at the helper function suggested as part of #1865. Which is a bit overused here as you only need to use a guard or isIterationCall, not both. Also it's a breaking change and I'm not in favor of it.

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.

None yet

5 participants