tiberiuc / redux-react-firebase

Use Firebase with React and Redux in ES6
MIT License
248 stars 36 forks source link

dataToJS only works for explicitely defined paths #50

Open Domiii opened 7 years ago

Domiii commented 7 years ago

Assuming I have:

@firebase((props, firebase) => ([
  '/quizzes',
  '/quizProblems'
]))

and I have an object /quizzes/abc in the database,

then:

dataToJS(firebase, '/quizzes/abc')

will return undefined.

However:

dataToJS(firebase, '/quizzes/').abc

works.

I would assume this is because you are not actually converting nested Firebase data into immutables. I absolutely agree that this is a good thing - in terms of performance, as well as in practical terms, considering that you are not supposed to change firebase-managed data directly at all, but rather commit any changes through the firebase API instead.

Either way, this and general data management guidelines should be explicitly stated in the docs, while also hinting at _.get to directly access firebase data at a given path.

Getting the redux mirror of the local firebase cache to become readonly, would be an even bigger plus! :)

RahavLussato commented 7 years ago

@domiii dataToJs searching is working only on the path you listen on, the change that you can't liten with it deeper then that is becayse my PR, it's because we inject "data" key on the listen path and search for it on dataToJs, this give us the ability to have multiple listeners on same sub tree of firebase without overriding one each other when component will unmount and clean the state. If you want to have this ability I'm not sure but pathToJs is not stricted so its need to work but you need to add data node on the string you search for like: "/data/quizzes/data/abc", we can also write new helper for deep search on dataToJs.

RahavLussato commented 7 years ago

@Domiii is this worked for you ?

Domiii commented 7 years ago

I am saying that this should be covered in the docs, and ideally in the samples! Hah :)

Tbh, the current state of the library is far from production (or maybe I'm just a bit too stupid). But I also need to admit that I was fairly new to React, Redux and Firebase, just the whole toolchain, so that woujld explain it!

I guess, the library is great for simple stuff and given you are already fairly familiar with all parts involved. For someone like me who wanted to get a React App w/ Firebase going, and keep the code nice and clean, I had to spend quite a few days on getting things to the point I wanted it. I am about to start test-running my code. It also comes with a clearer approach to getting the data you want, no matter where it is located, through a high-level wrapper object around any path you use.

You mentioned that pathToJs can be used to get any data anywhere without any restrictions, but need to add the data prefix to the path? Maybe at the very least that can be documented, and _.get should maybe also be mentioned! :)

RahavLussato commented 7 years ago

@Domiii i agree with you about the docs, they far from being good, and most of it is because of me, i'm doing changes without updating the docs. i'm not agree with the part of the current state of the lib, i'm using it on production on more then one project and its working perfect, its also giving you ability to do stuff without special implementations more then the firebase SDK (look at my last PR #47). i didn't understood how _.get is related to here ? i can tell you that i started to use this lib because it was the only lib that combining redux-react-firebase and from then i contributed a lot of changes evert time that i needed something that wasn't exists.

i'm happy that you are using this lib and i think that you'll find it useful, if you want a little tip from me, to be honest i didn't went over the docs, i just started to look in the code, the code is very simple and you'll find out its much faster to understand it from the code :)