prescottprue / react-redux-firebase

Redux bindings for Firebase. Includes React Hooks and Higher Order Components.
https://react-redux-firebase.com
MIT License
2.55k stars 559 forks source link

Conflict with redux-persist #890

Open alexander-ding opened 4 years ago

alexander-ding commented 4 years ago

What is the current behavior? The homepage of my app listens to a document with Timestamp data. The listener populates the store with data such as

seconds: 1567396800
nanoseconds: 0
__proto__:
  toDate: ƒ ()
  toMillis: ƒ ()
  U: ƒ (t)
  isEqual: ƒ (t)
  toString: ƒ ()
  valueOf: ƒ ()
  constructor: ƒ t(t, n)
  __proto__: Object

However, using redux-persist to persist the store causes it to store the serialized Timestamp object

{  
  seconds: 1567396800,
  nanoseconds: 0,
}

which does not have access to the methods.

This means that every time I refresh the page, after redux-persist rehydrates the serialized timestamp, before the firebase listener responds, the program would have a populated Timestamp object that doesn't actually have any of the methods.

This is problematic. Take this dummy component

const CurrentStatus = ({terms}) => {
  if (!isLoaded(terms)) return "loading"
  const date = terms.fallStart.toDate()
  /* displays the date 8?
}

and connect it to firebase to listen for terms. In that brief moment fallStart.toDate is not defined, and React will throw TypeError: terms.fallStart.toDate is not a function.

The workaround is pretty simple: const date = new Date(terms.fallStart.seconds * 1000), but this kind of code is a little gross. I'm not really sure what could be done about it, but I thought it's helpful to at least be aware of this.

Which versions of dependencies, and which browser and OS are affected by this issue? Did this work in previous versions or setups? "react-redux-firebase": "^3.1.2", "redux-persist": "^6.0.0",

prescottprue commented 4 years ago

Thanks for reaching out - this makes sense due the the serializing you mentioned. Not really sure about how this would be addressed other than the workaround mentioned