Feature/dynamic dom

[jennasalau]

Hey guys,

If you can cast your eyes over this one I would appreciate it for my own sanity. If nothing jumps out at you please give it a tick of approval so I feel slightly more comfortable merging it :)

The bulk of the work has been to the Bootstrapper class.

This PR address issue number #12 by adding a new update() method as well as implementing an automatic dom watcher via the MutationObserver api.

Watcher performance looks good and everything still pretty snappy. I've tested in IE9 & 10 with a polyfill and again impressed with performance.

Im not sure if the watcher should be enabled by default though?.. on one part I like that "it will just work" out of the box. However, theoretically it will perform better with the watcher disabled and you manually call update() after a dom change (which would be my recommendation for serious web apps). What default do you think is better?

[dkeeghan]

Looks like a good fix. In regards to your question about the watcher can you make it configurable to enable or disable if you choose? I agree for most larger apps it would be smarter to run it as needed but to make it easier for people who aren't as familiar with React it might be good to have it there.

[keeganstreet]

Looks good! Small suggestion - I would prefer if we import the MutationObserver polyfill via npm, and then self-host it, rather than loading it from a third party CDN. Otherwise we are completely trusting that CDN not to execute malicious code on our sites. It is an unnecessary security vulnerability.

[jennasalau]

Thanks for the feedback 👍

@dkeeghan yes I've made it configurable so you can opt in to switch it off with this.enableWatcher = false;

I did have a thought this morning it might be better to have that passed in as an option when you set the container.

Example maybe this is better

this.setContainer(/*container=* /myContainer, /*enableWatcher=*/ false)

@keeganstreet good pick up and reasoning.. this is me being lazy. I'll get that updated to our best practice :)

[jeffdowdle]

Personally I think disabling the watcher by default is the way to go. But I can see the argument for enabling it too.

The way I see it, as someone new to using React Habitat, I'd probably take these steps:

1. Get my app/containers working on my static HTML.
2. Live happily for a while, possibly forever.
3. I realise I need to re-render my component for whatever reason.
4. I search the React Habitat docs for 'dynamic html' or something similar. Then I follow the instructions there.

I would expect most people to stop at step 2, and this way they all get the performance benefits without having to know about the watcher option.

[jennasalau]

Okay I've implemented everyones feedback. I've ended up agreeing with @jeffdowdle and I actually think disabled by default works much better now. As soon as I did that all my unit tests where much easier to write too. I was slightly worried that developers might complain that it chugs on large apps with the watcher, at least this way they are making conscious decisions and I will sleep easier.

@keeganstreet No more CDN's in the examples :)

The docs are starting to get huge. Not sure what we can do about that, but might have a review later on.

Does this look okay to everyone for a merge?

[dkeeghan]

Looks good. Nice one! Yep, we should look at a static site using GitHub.io as a standard docs format when they get large like this.