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

Error when freeze and ignoreIfNoChange both true #11

Closed
ariutta opened this issue May 31, 2017 · 2 comments
Closed

Error when freeze and ignoreIfNoChange both true #11

ariutta opened this issue May 31, 2017 · 2 comments

Comments

@ariutta
Copy link

ariutta commented May 31, 2017

Should it be possible to set both freeze and ignoreIfNoChange to true?

When I run the following code using version 1.0.34:

var iassign = require("immutable-assign");

iassign.freeze = true;
iassign.ignoreIfNoChange = true;

var map1 = { a:1, b:2, c:3 };

// 1: Calling iassign() to update map1.b, using overload 2
var map2 = iassign(
    map1,
    function (m) { m.b = 50; return m; }
);

I get this error:

.../node_modules/immutable-assign/deploy/iassign.js:75
                newValue = setProp(obj);
                           ^
TypeError: Cannot assign to read only property 'b' of object '#<Object>'
@engineforce
Copy link
Owner

engineforce commented Jun 1, 2017

When ignoreIfNoChange is true, setProp() is called early with the input object (which is not the copy and is frozen), and its return value is compared with input object using ===. If no change occurred, iassign() can return early, improving performance.

There are two options to solve your problem:

  1. Use getProp() to the extract the exact property you want to set (preferred):
var map2 = iassign(
    map1,
    function (m) { return m.b; },
    function (b) { return 50; }
);
  1. Use Object.assign() to do the manual copy
var map2 = iassign(
    map1,
    function (m) { return Object.assign({}, m, { b: 50 }); }
);

You should use ignoreIfNoChange if you don't know if the setProp() will update your object or not, e.g., a reducer calling another reducer:

function AppReducer(state, action) {
    state = iassign(
        state,
        function(s) { return s.count; },
        function(c) { return CountReducer(c, action); },
        undefined,   // no context
        { ignoreIfNoChange: true });

    return state;
}

function CountReducer(count, action) {
    switch (action.type) {
    {
        case "INCREMENT": count++; break;
        case "DECREMENT": count--; break;
    }  
    return count;  // it will not change on other actions.
}

@ariutta
Copy link
Author

ariutta commented Jun 1, 2017

That sounds reasonable. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants