Yomguithereal / baobab-react

React integration for Baobab.
MIT License
310 stars 38 forks source link

Sharing a tree between windows #123

Open nbermudezs opened 7 years ago

nbermudezs commented 7 years ago

Hi @Yomguithereal!

I've been using baobab-react for a few months now and I was recently asked for some interesting feature. Lets imagine that I'm writing a rich text-editor that can either be embedded (iframed) in a page or shown in a pop-out window. I was asked that if you start using the embedded editor you should be able to switch to a pop-out view and resume where you left and vice versa. When you switch to pop-out there are still some elements that are shown in the 'embedded' view and will react to state changes.

Yes, you can serialize the tree and pass it down, deserialize and build a new tree based on that state. And then every time something changes in the store you have to notify the change to the embedded view.

I was not a fan of this idea so what I tried was to expose the state tree in the window scope and have the pop-out access that tree instead of having its own new tree. This looks like

let parentStateTree;

if (!isEmbedded()) { // it is a pop-out
  try {
    parentStateTree = window.opener.StateTree;
  } catch(e) {}
}

const StateTree = parentStateTree || new Baobab({...});
if (isEmbedded()) {
  window.StateTree = StateTree;
}
export default StateTree;

Besides that I just needed to change two lines of baobab-react to have two different interfaces listen to the same store. Pretty awesome surprise.

Now, those two lines are related to this What I had to do was:

function root(tree, Component, BaobabClass = Baobab) {
  if (!(tree instanceof BaobabClass))
...

This was necessary because, even though the tree was an instance of a Baobab class, the instance was created in a different window and therefore with a different reference to 'baobab'.

I really liked how easy it was to accomplish this and if you think this is of benefit for someone else I'm happy to submit a PR with this change.

Sorry if the message ended up too long, I thought some context was necessary 😄

Yomguithereal commented 7 years ago

Hum, intuitively I'd say it's usually dangerous to make different context access the same references but why not. Ideally, the baobab library should offer some kind of way to detect baobab instances and whatnot using something similar as Array.isArray (which solves the same kind of issues) rather than relying on instanceof indeed. But I guess your fix should also work. What do you think?

nbermudezs commented 7 years ago

I agree. Something similar to Array.isArray would be nice. We can start a conversation there. In the meantime, should we get my change in here?

Yomguithereal commented 7 years ago

I think we can do better: we can drop the baobab dependency and require this as a peer dependency and rely on this internal property here:

https://github.com/Yomguithereal/baobab/blob/master/src/baobab.js#L111

Which actually was made to tackle this kind of issues.

nbermudezs commented 7 years ago

Do you mean doing something like

function root(tree, Component) {
  if (!tree || tree.toString() !== '[object Baobab]')
...

or maybe

function root(tree, Component) {
  if (!tree || tree.toString() !== (new Baobab()).toString())
...

to not hardcode the string in there.