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

Is it possible to get mutable data out of icepick? #7

Closed
bitinn opened this issue Jun 9, 2015 · 11 comments
Closed

Is it possible to get mutable data out of icepick? #7

bitinn opened this issue Jun 9, 2015 · 11 comments

Comments

@bitinn
Copy link

bitinn commented Jun 9, 2015

There seem to be 2 camp on immutable object:

  • assume you want the native array/object method instead of wrapping them as immutable api (for better compatibility with other api that assume mutable array/object)
  • assume you want to use immutable object all the way, if you happen to need mutable array/object, there are ways to convert immutable object back to mutable.

I believe icepick is in the first camp (as opposed to immutable.js or seamless-immutable, which are likely in the 2nd camp).

Am I right in thinking icepick assume developers will pass its instance directly? Because I see no method to extract mutable data out of it.

@aearly
Copy link
Owner

aearly commented Jun 9, 2015

You are correct that icepick is in the first camp. One thing to note, though, is that there are no "instances" -- icepick only returns plain (but frozen) objects that can be used with anything that doesn't try to modify them (direct property access, arr.map(), etc..). seamless-immutable is not quite in the second camp -- it still returns plain objects, but adds a few more methods to them for convenience.

If you want mutable data back out, they way to get it is to deeply clone the object. You can use something like Lodash _.cloneDeep or even JSON.parse(JSON.stringify(obj)).

Would a i.thaw() function that does this be helpful?

@bitinn
Copy link
Author

bitinn commented Jun 9, 2015

Having an api would help, thaw sounds like a good name.

I just realize once we freeze an object there is no unfreeze, we can only clone it to a new one. It would seem that thaw then mutate then freeze again will have roughly the same performance as diffing (eg. assoc etc.) the immutable itself (which does the same thing)?

If so, perhaps we should just diff the immutable itself, and avoid thaw most of the time.

@aearly
Copy link
Owner

aearly commented Jun 9, 2015

What do you mean by "diff the immutable"? The proper way to see if things have changed would be to see if objects are still reference-equal to their previous values.

@bitinn
Copy link
Author

bitinn commented Jun 9, 2015

My wording was probably wrong, sorry about that. I simply meant something like newColl = i.assoc(coll, "b", 3). And I was talking about how icepick would be updating b in this case.

If I understand correctly:

  • Say coll is frozen already, when you are creating the newColl, you have to copy coll's value into a mutable object again, and then change b, and then freeze it again.
  • But unlike the initial i.freeze(coll), we don't have to freeze any data recursively, only the object that has changed, ie. the object containing b.
  • So this is faster than i.thaw(coll); coll.b = 3; i.freeze(coll); particularly for larger coll, because we don't need to freeze other part of it.

True?

If so then we shouldn't use thaw/freeze for mutation, we should try to change coll into a newColl with assoc, which makes thaw less desirable.

TL;DR: i.thaw is nice to have as a helper function, but probably shouldn't be used for mutation.

@aearly
Copy link
Owner

aearly commented Jun 9, 2015

Yep, I think you understand. If you look at the implementation of i.assoc that's essentially what it does -- (shallow)copy, modify, freeze.

@bitinn
Copy link
Author

bitinn commented Jul 17, 2015

I have been using icepick in production for a month now and things are working great, though I did find i.thaw to be a useful addition, imagine this scenario:

Let say we are using a virtual-dom rendering system that allow template to be rendered both server-side and client-side. Now imagine we are to render a list of tweets.

Because data from backend are just normal object, we are doing something like this:

    var tweets = tweets.map(function(tweet, i) {
        return tweetView(tweet);
    });

Say we have a variable that's global, but is needed when rendering tweetView:

    var domain = 'twitter.com';
    var tweets = tweets.map(function(tweet, i) {
        tweet.domain = domain;
        return tweetView(tweet);
    });

On server-side this is fine, but on client-side our model is an icepick-powered store, which means tweet.domain = domain; will fail.

At this point we have a few options:

  1. Make server use icepick as well, which doesn't make much sense as data is used only once.
  2. Do an Object.isForzen check, and set data differently. This works but expose icepick api to virtual-dom template, which feel kinda weird.
  3. i.thaw, modify and pass the data as normal object. Unfortunately we will take a performance penalty here from clone.

Do you see any better approach?

@aearly
Copy link
Owner

aearly commented Jul 17, 2015

Could you do something like:

    var domain = 'twitter.com';
    var tweetViews = tweets.map(function(tweet, i) {
        return tweetView(i.assoc(tweet, "domain", domain));
    });

The update to the tweet would be discarded, but it would work. The other option would be to update all your tweets with it:

var tweets = i.map(tweets, function (tweet) {
  return i.assoc(tweet, "domain", domain);
});

// and then

var tweetViews = tweets.map(function(tweet, i) {
    return tweetView(tweet);
});

@bitinn
Copy link
Author

bitinn commented Jul 18, 2015

One thing I am not sure about: on server-side my tweets is a plain array that's not frozen, does i.map and i.assoc still work?

@aearly
Copy link
Owner

aearly commented Jul 18, 2015

Yes. It'll work with any plain JS Array or Object, and they don't have to be frozen.

@aearly aearly closed this as completed in 0c15177 Jul 20, 2015
@bitinn
Copy link
Author

bitinn commented Jul 20, 2015

Wow cheers, would i.assoc actually be faster than i.thaw in my use case though? Both of them do clone but i.thaw will have to do it recursively.

@aearly
Copy link
Owner

aearly commented Jul 20, 2015

Yes, assoc would be faster since it's a shallow clone.

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

No branches or pull requests

2 participants