Whoops

buggy-coffee.js

/* jasmine-matcher-wrapper - 0.0.2
 * Wraps Jasmine 1.x matchers for use with Jasmine 2
 * https://github.com/testdouble/jasmine-matcher-wrapper
 */
(function() {
  var __slice = [].slice,
    __hasProp = {}.hasOwnProperty;

  (function(jasmine) {
    var comparatorFor;
    if (jasmine == null) {
      return typeof console !== "undefined" && console !== null ? console.warn("jasmine was not found. Skipping jasmine-matcher-wrapper. Verify your script load order.") : void 0;
    }
    if (jasmine.matcherWrapper != null) {
      return;
    }
    comparatorFor = function(matcher, isNot) {
      return function() {
        var actual, context, message, params, pass, _ref;
        actual = arguments[0], params = 2 <= arguments.length ? __slice.call(arguments, 1) : [];
        context = {
          actual: actual,
          isNot: isNot
        };
        pass = matcher.apply(context, params);
        if (isNot) {
          pass = !pass;
        }
        if (!pass) {
          message = (_ref = context.message) != null ? _ref.apply(context, params) : void 0;
        }
        return {
          pass: pass,
          message: message
        };
      };
    };
    return jasmine.matcherWrapper = {
      wrap: function(matchers) {
        var matcher, name, wrappedMatchers;
        if (jasmine.addMatchers == null) {
          return matchers;
        }
        wrappedMatchers = {};
        for (name in matchers) {
          if (!__hasProp.call(matchers, name)) continue;
          matcher = matchers[name];
          wrappedMatchers[name] = function() {
            return {
              compare: comparatorFor(matcher, false),
              negativeCompare: comparatorFor(matcher, true)
            };
          };
        }
        return wrappedMatchers;
      }
    };
  })(jasmine || getJasmineRequireObj());

}).call(this);

buggy.coffee

((jasmine) ->
  return console?.warn("jasmine was not found. Skipping jasmine-matcher-wrapper. Verify your script load order.") unless jasmine?
  return if jasmine.matcherWrapper?

  comparatorFor = (matcher, isNot) ->
    (actual, params...) ->
      context = {actual, isNot}
      pass = matcher.apply(context, params)
      pass = !pass if isNot
      message = context.message?.apply(context, params) unless pass
      {pass, message}

  jasmine.matcherWrapper =
    wrap: (matchers) ->
      return matchers unless jasmine.addMatchers?
      wrappedMatchers = {}
      for own name, matcher of matchers
        wrappedMatchers[name] = ->
          compare: comparatorFor(matcher, false)
          negativeCompare: comparatorFor(matcher, true)
      wrappedMatchers

)(jasmine || getJasmineRequireObj())

working-coffee.js

/* jasmine-matcher-wrapper - 0.0.3
 * Wraps Jasmine 1.x matchers for use with Jasmine 2
 * https://github.com/testdouble/jasmine-matcher-wrapper
 */
(function() {
  var __hasProp = {}.hasOwnProperty,
    __slice = [].slice;

  (function(jasmine) {
    var comparatorFor, createMatcher;
    if (jasmine == null) {
      return typeof console !== "undefined" && console !== null ? console.warn("jasmine was not found. Skipping jasmine-matcher-wrapper. Verify your script load order.") : void 0;
    }
    if (jasmine.matcherWrapper != null) {
      return;
    }
    jasmine.matcherWrapper = {
      wrap: function(matchers) {
        var matcher, name, wrappedMatchers;
        if (jasmine.addMatchers == null) {
          return matchers;
        }
        wrappedMatchers = {};
        for (name in matchers) {
          if (!__hasProp.call(matchers, name)) continue;
          matcher = matchers[name];
          wrappedMatchers[name] = createMatcher(name, matcher);
        }
        return wrappedMatchers;
      }
    };
    createMatcher = function(name, matcher) {
      return function() {
        return {
          compare: comparatorFor(matcher, false),
          negativeCompare: comparatorFor(matcher, true)
        };
      };
    };
    return comparatorFor = function(matcher, isNot) {
      return function() {
        var actual, context, message, params, pass, _ref;
        actual = arguments[0], params = 2 <= arguments.length ? __slice.call(arguments, 1) : [];
        context = {
          actual: actual,
          isNot: isNot
        };
        pass = matcher.apply(context, params);
        if (isNot) {
          pass = !pass;
        }
        if (!pass) {
          message = (_ref = context.message) != null ? _ref.apply(context, params) : void 0;
        }
        return {
          pass: pass,
          message: message
        };
      };
    };
  })(jasmine || getJasmineRequireObj());

}).call(this);

working.coffee

((jasmine) ->
  return console?.warn("jasmine was not found. Skipping jasmine-matcher-wrapper. Verify your script load order.") unless jasmine?
  return if jasmine.matcherWrapper?

  jasmine.matcherWrapper =
    wrap: (matchers) ->
      return matchers unless jasmine.addMatchers?
      wrappedMatchers = {}
      for own name, matcher of matchers
        wrappedMatchers[name] = createMatcher(name, matcher)

      wrappedMatchers

  createMatcher = (name, matcher)  ->
    ->
      compare: comparatorFor(matcher, false)
      negativeCompare: comparatorFor(matcher, true)

  comparatorFor = (matcher, isNot) ->
    (actual, params...) ->
      context = {actual, isNot}
      pass = matcher.apply(context, params)
      pass = !pass if isNot
      message = context.message?.apply(context, params) unless pass
      {pass, message}

)(jasmine || getJasmineRequireObj())

Ran into an issue today with how aggressively CoffeeScript pulls up variable declarations.

The issue starts with this snippet of CoffeeScript. I can't rely on Underscore or ES5 looping here, so I settled for a for-loop (which I don't really use often at all):

      wrappedMatchers = {}
      for own name, matcher of matchers
        wrappedMatchers[name] = ->
          compare: comparatorFor(matcher, false)
          negativeCompare: comparatorFor(matcher, true)
      wrappedMatchers

This will compile down to this snippet:

        var matcher, name, wrappedMatchers;
        //...
        wrappedMatchers = {};
        for (name in matchers) {
          if (!__hasProp.call(matchers, name)) continue;
          matcher = matchers[name];
          wrappedMatchers[name] = function() {
            return {
              compare: comparatorFor(matcher, false),
              negativeCompare: comparatorFor(matcher, true)
            };
          };
        }
        return wrappedMatchers;

Which, because CoffeeScript pulled matcher's variable declaration to the top of the function, means that matcher will only ever point to one function (the one most recently set, or rather, the last one iterated over).

The effect of this is that if I were to call:

jasmine.matcherWrapper.wrap({
  toBeFoo: -> "A",
  toBeBar: -> "B"
})

Any instances of:

expect(thing).toBeFoo()

Would actually invoke function "B". Not exactly ideal.

The easiest fix was just to yank out another function to prevent the reference from being identical across every property in the object, like this snippet shows:

...
      for own name, matcher of matchers
        wrappedMatchers[name] = createMatcher(name, matcher)

      wrappedMatchers

  createMatcher = (name, matcher)  ->
    ->
      compare: comparatorFor(matcher, false)
      negativeCompare: comparatorFor(matcher, true)

And that prevents the variable from being declared ahead of the scope I meant for it.

I don't know what to take from this, because it means I've been using CoffeeScript for two whole years now and this is the first time I've run into a problem with its scoping. I guess my takeaway is "don't reach for for loops when locally assigning stuff"?

Oh, worth pointing out that setting a variable inside of the for-loop to the value of matcher like this:

      for own name, matcher of matchers
        localMatcher = matcher
        wrappedMatchers[name] = ->

Also doesn't work. CoffeeScript will pull up localMatcher in the same way it pulled up matcher

Ah, I have had this issue before. I've started using underscore/lodash looping to prevent this kind of scoping issue. Not ideal, but it works because the inner loop is a function callback.