bfitch / cerebral-falcor-module

Cerebral integration for Netfilx's Falcor
6 stars 2 forks source link

Diffing circular references #1

Open ekosz opened 8 years ago

ekosz commented 8 years ago

Looking at the code it doesn't seem like this library will handle circular references well.

For example:

cache = {
   usersById: {
    1: {
      goal: { $type: 'ref', value: ['goalsById', 1] }
    }
  },
  goalsById: {
    1: {
      user: { $type: 'ref', value: ['usersById', 1] }
    }
  }
};

When this is the case using deep-diff should cause an infinite loop. This is because falcor-expand-cache will expand forever. Following the references each time.

That being said, I didn't test this and this is only from my initial overview of the code. Do you know if this is an issue or not?

Cheers, Eric

bfitch commented 8 years ago

Hey Eric, thanks for brining this to my attention. I'm a sorry to say that I didn't even consider that circular reference problem. I haven't encountered it in my little demo but, yeah, obviously need to handle that.

Did you find a solution for redux?

ekosz commented 8 years ago

@bfitch Yep, I have a flight from NYC to SF in a few minutes and I'm going to be writing up the new documentation on redux-falcor@3.0. You can splunk through the source code, but basically it does a top down rendering on every change. This isn't as performant as what you're going for though as there is no patching of the atom.

I think there is a solution out there for that, but I wanted to focus on utility first rather than performance. I'm thinking of using a the falcor version tag their values have to only patch those values whose version number has changed. @trxcllnt created a great library called reaxtor which is very neat and tackles the problem from a different perspective. I believe his library does the falcor version diffing.

ekosz commented 8 years ago

Update: I've now pushed and updated the documentation on redux-faclor

bfitch commented 8 years ago

Very cool @ekosz! I took a look at the source for FalcorProvider but I'm not seeing how it solves the circular ref problem. Even when replacing all state on each change, won't expandCache() need to bailout when expanding falcor.getCache() in that case somehow? Apologies if I'm being dense...just not clicking yet. Really appreciate your time.