florinn / typemoq

A simple mocking library for TypeScript
MIT License
428 stars 45 forks source link

Proxy.isProxy() throws an error when trying to clone an object with a null value (via StaticMock.cloneDeep()) #32

Closed ajeffrey closed 8 years ago

ajeffrey commented 8 years ago

The proxy.isProxy() function doesn't check for null before checking for the __id string in an object (when calling .Mock.ofInstance).

Here's a rough replication of the issue in raw JS:

function cloneDeep(target) {
        var copy = target;
        if (!_.isFunction(target)) {
            var func = function (x) {
                var value;
                if (isProxy(x))
                    return x;
            };
            copy = _.cloneDeepWith(target, func);
        }
        return copy;
}

function isProxy(obj) {
        console.log(obj);
        if (!_.isUndefined(obj) &&
            !_.isUndefined(obj['__id']) && obj['__id'] === 'test')
            return true;
        else
            return false;
}

cloneDeep({
    a: {
        b: {
            c: null
        }
    }
});

and here's the backtrace I get (against 1.0.1):

TypeError: Cannot read property '___id' of null

      at Function.Proxy.isProxy (node_modules/typemoq/dist/typemoq.js:530:31)
      at func (node_modules/typemoq/dist/typemoq.js:1208:29)
      at baseClone (node_modules/lodash/lodash.js:2667:27)
      at node_modules/lodash/lodash.js:2721:34
      at arrayEach (node_modules/lodash/lodash.js:537:11)
      at baseClone (node_modules/lodash/lodash.js:2715:7)
      at Function.cloneDeepWith (node_modules/lodash/lodash.js:11120:14)
      at Function.StaticMock.cloneDeep (node_modules/typemoq/dist/typemoq.js:1211:22)
      at Function.StaticMock.ofInstance (node_modules/typemoq/dist/typemoq.js:1190:37)
      at Function.MockApi.ofInstance (node_modules/typemoq/dist/typemoq.js:1274:31)

I suspect the fix would be to replace the _.isUndefined call with something that only proceeds if obj is a valid non-null object.

ajeffrey commented 8 years ago

just tested in the dist js and changing !_.isUndefined(obj) to typeof obj === 'object' && obj does the trick.

florinn commented 8 years ago

Great. Could you put together a PR with the fix?

ajeffrey commented 8 years ago

will do

florinn commented 8 years ago

I've added the null check, let me know if you run into other issues.

ajeffrey commented 8 years ago

oh ok, was going to submit a PR today but thanks :)