Cursor exception in deepFreeze() on IE

[newyankeecodeshop]

Object.isFrozen throws an exception when called with 'null'. Use an isObject() function that tests for null.

Also, ignore functions because they can have recursive members (e.g. constructors).

[lijunle]

I think you could want to remove lodash.isobject from package.json too.

[dustingetz]

Why not use _.isObject? Also what is your use case for storing functions in a cursor, that is forbidden

[dustingetz]

see https://github.com/dustingetz/react-cursor/issues/19

[newyankeecodeshop]

I didn't use _.isObject because it returns true for both objects and functions. I understand that storing functions in state is not desirable. However, in some cases we use objects with a particular prototype:

function User(data) {
    this.name = data.name;
}

class MyComponent extends React.Component {
    constructor() {
        this.state = { user: new User({ name: 'Andrew' }), userType: User };
}

In the above example, user objects will have a prototype that points to User, and the User function has a constructor property which is itself. So the recursion breaks deepFreeze() if it tries to process functions. Moreover, if you want to forbid using objects with functions anywhere, you should implement a check in the Cursor constructor, and not rely on deepFreeze() throwing an exception on IE. :-)

[dustingetz]

1) I'm okay with adding the assertion to disallow functions - but will that break your use case? 2) can you use _.isObject && !_.isFunction ?

[newyankeecodeshop]

1) Yes I think so. It prevents us from storing an object type in our state. (i.e. a property on state that is the function to use when instantiating new values - "new this.state.userType()") 2) I was just trying to fix the exception. If you're excited to keep using _.isObject, then by all means go for it. I just figured that deepFreeze doesn't really want to pass functions to Object.isFrozen() or Object.freeze() anyway. 2a) The function you want is really more along the lines of "isFreezable()", which is not the same as "isObject".

[dustingetz]

if you implement isFreezable with lodash fns and the tests pass i will merge it. I don't want to use an adhoc impl of isObject due to scary comments in the lodash code's implementation https://github.com/lodash/lodash/blob/4.0.0/lodash.js#L9658

[dustingetz]

isFunction is scary too https://github.com/lodash/lodash/blob/4.0.0/lodash.js#L9571

[newyankeecodeshop]

FYI, you're using the "omit-keys" library, which includes a non-lodash implementation of isObject - see /omit-keys/node_modules/isobject/index.js.

[dustingetz]

thanks, opened https://github.com/dustingetz/react-cursor/issues/88

[newyankeecodeshop]

Would you accept a change that takes your original code and just puts a !!prop in there?

if (!!prop && typeof prop == 'object' && !Object.isFrozen(prop)) {
    deepFreeze(prop);
}

I did the above originally, and then noticed that there was no point in having different logic between that if statement and the check on line 110. So I made the custom function. That was how I arrived at this pull request.

[dustingetz]

Yes i will merge that (and probably change it later now that it has my attention), sorry for being picky, this code comes under a lot of scrutiny on social media so i try to give people as few bikeshed details to complain about as possible