Skip to content

Commit

Permalink
Fix issue when one of the selector is null
Browse files Browse the repository at this point in the history 
When one of the selector is null, the next callback was not called => xray would then never call the fn function. Here, if the selector v is null (or undefined or other, basically, not string/function/array), the function just calls the next callback anyway.
  • Loading branch information
dfdeagle47 committed Jul 10, 2015
1 parent 5a8f05f commit e7318d5
Showing 1 changed file with 4 additions and 3 deletions.
7 changes: 4 additions & 3 deletions index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,15 +172,15 @@ function Xray() {
walk(selector, function(v, k, next) {
if ('string' == typeof v) {
var value = resolve($, root(scope), v);
next(null, value);
return next(null, value);
} else if ('function' == typeof v) {
v($, function(err, obj) {
if (err) return next(err);
next(null, obj);
return next(null, obj);
});
} else if (isArray(v)) {
if ('string' == typeof v[0]) {
next(null, resolve($, root(scope), v));
return next(null, resolve($, root(scope), v));
} else if ('object' == typeof v[0]) {
var $scope = $.find ? $.find(scope) : $(scope);
var pending = $scope.length;
Expand All @@ -197,6 +197,7 @@ function Xray() {
});
}
}
return next();
}, function(err, obj) {
if (err) return fn(err);
fn(null, obj, $);
Expand Down

2 comments on commit e7318d5

@bdentino
Copy link
Contributor

Choose a reason for hiding this comment

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

@dfdeagle47 the case where v is a function actually doesn't return within its if block, so it falls out and calls next() at the end of this function which causes some other problems. Can you verify that this wasn't intentional? I'm catching this in https://github.com/lapwinglabs/x-ray/tree/bugfix/nested-crawling but don't want to merge if it's going to re-break something you already fixed

@dfdeagle47
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, just saw your message...

You're right, I think it's a mistake on my part, it should probably be

return v($, function(err, obj) {
            if (err) return next(err);
            return next(null, obj);
          });

The thing is, I haven't used the case where v is a function, so I didn't catch it, but I agree with you that it could cause problems if next() is called twice.

Please sign in to comment.