[WIP] Immutable.js Support

[JustinTulloss]

I don't want to merge this patch as-is, but if something like this looks reasonable, I'd like to adapt this patch to provide a parallel API that works if Immutable.js is present in the environment. Maybe bindAsList and bindAsOrderedMap would come into existence next to the existing bind functions if Immutable.js was present.

Summary

ReactFireMixin can be rather slow with many items causing many change
events which can trigger many rerenders. By using Immutable data
structures we can cheaply check the entire tree and only rerender the
components that have actually changed their data.

This patch makes bindAsObject bind an Immutable.OrderedMap to the specified
prop and bindAsArray bind an Immutable.List to the specified prop.

It also changes the tests to verify that the data returned is correct.

Finally, the patch adds a boundVarsHaveUpdated method that will
cheaply check for changed state in the bound state variables. It's
intended to be used in a more complex shouldComponentUpdate method.
Tests for the new method are added.

[coveralls]

Coverage decreased (-1.13%) when pulling 009c993 on JustinTulloss:immutable into cad4742 on firebase:master.

[brigand]

Throw/log a helpful error it's undefined.

var immutableCdn = "https://cdnjs.cloudflare.com/ajax/libs/immutable/3.4.1/"
throw new TypeError(
  'Immutable is undefined.  Please include it in your page.   '
  + 'You can get it at ' + immutableCdn + 'immutable.js or ' + immutableCdn + 'immutable.min.js'
);

[JustinTulloss]

Should an error get thrown or should just work as it does today?

[brigand]

The problem is that in CommonJS/AMD it'll just fail unless you supply a fake version of Immutable with exports undefined. So the browser would behave differently in that it just works with or without it.

UMD dependencies are tricky, and this is going to require a good guide for each tool people might use (none, browserify, webpack, RequireJS).

Edit: actually I'm not sure how to do this in browserify. It's tricky to modify how things in node_modules behave with browserify.

[nelix]

I really like this idea, I'd love it if it were a little more abstract so I could use it in my actions/stores...

[JustinTulloss]

That might be possible. There could be a small wrapper library that wraps
the official firebase client with immutable structures, then this library
could use that.

The best way would be to implement this in the actual firebase client
library, but that doesn't seem to be possible.

On Mon Jan 12 2015 at 2:45:36 PM Nathan Hutchision notifications@github.com
wrote:

I really like this idea, I'd love it if it were a little more abstract so
I could use it in my actions/stores...

—
Reply to this email directly or view it on GitHub
#30 (comment).

[coveralls]

Coverage decreased (-0.98%) to 97.3% when pulling 391dad3 on JustinTulloss:immutable into cad4742 on firebase:master.

[jwngr]

Okay, lots to comment on here. First off, thanks for the PR! Second off, sorry for the huge delay in responding. I noticed that this repo was getting a little outdated and wanted to jump in and see what we can do here.

Let me preface the remainder of what I'm going to write by saying that I'm not terribly familiar with Immutable and this is my first real deep dive into it. I also have not been staying as up-to-date with React as I would like to. Immutable looks like a cool library, but I am wary to add this seemingly unnecessary dependency to ReactFire. I especially don't want to make it required. Maybe you can talk me through some of the benefits of this library in the context of ReactFire. It seems that Firebase already supports immutable snapshots and I'm not sure how adding in the Immutable library makes things better. I assume Immutable + React does something special which causes us not to re-render as often, but I don't really understand how that works. Could you enlighten me on that topic?

Depending on how this conversation goes as I learn more about Immutable, I could see up publishing an Immutable-compatible version of ReactFire alongside the default non-Immutable version. I'm looking forward to hearing back!

[JustinTulloss]

Hey Jacob,

Thanks a lot for the comments. The reason I presented this the way I did
was to just throw out the minimal amount of effort I could to see if there
was a conversation to be had. It seems that there is, which is great. I
definitely don't want to add Immutable.js as a dependency for ReactFire and
would instead try to make this an optional thing.

The things that I'm trying to accomplish are this:

• When bindAsObject calls setState, the new object is indeed a new
  object (=== is false).
  • When a user of the data object (the react component) updates the data,
    the underlying object does not update. They instead get a new reference.
    This means that they are free to update the application state without
    affecting the component state until they explicitly do so.

However, your point that data snapshots are basically immutable has gotten
me thinking about this differently. I think you could have a lot of the
benefits of Immutable.js with only a small amount of work in ReactFire and
perhaps some sugar outside of ReactFire. Kind of. I assume snapshot.val()
always returns the same object, so if one caller mutates that object, a
second caller will get the mutated object.

I'm not back in the office until Monday, but I hope to pull together a
proof of concept that will demonstrate what I'm thinking now, which might
be good enough without bringing in Immutable.js at all.

Thanks again for taking a look at this.

On Thu, Jan 29, 2015 at 10:14 PM, Jacob Wenger notifications@github.com
wrote:

Okay, lots to comment on here. First off, thanks for the PR! Second off,
sorry for the huge delay in responding. I noticed that this repo was
getting a little outdated and wanted to jump in and see what we can do here.

Let me preface the remainder of what I'm going to write by saying that I'm
not terribly familiar with Immutable and this is my first real deep dive
into it. I also have not been staying as up-to-date with React as I would
like to. Immutable looks like a cool library, but I am wary to add this
seemingly unnecessary dependency to ReactFire. I especially don't want to
make it required. Maybe you can talk me through some of the benefits of
this library in the context of ReactFire. It seems that Firebase already
supports immutable snapshots and I'm not sure how adding in the Immutable
library makes things better. I assume Immutable + React does something
special which causes us not to re-render as often, but I don't really
understand how that works. Could you enlighten me on that topic?

Depending on how this conversation goes as I learn more about Immutable, I
could see up publishing an Immutable-compatible version of ReactFire
alongside the default non-Immutable version. I'm looking forward to hearing
back!

—
Reply to this email directly or view it on GitHub
#30 (comment).

[JustinTulloss]

Closing this out for now

[jwngr]

Sorry for not responding. I really need to look at this repo more often...

I assume snapshot.val() always returns the same object, so if one caller mutates that object, a second caller will get the mutated object.

This is not true, it actually acts like this:

var a = snapshot.val();
a.foo = 'bar';
console.log(a === snapshot.val());  // false

I do think this issue should be closed and that DataSnapshots are exactly the immutable objects you want. Let me know if you've got other questions.

[JustinTulloss]

Sounds great, thanks!