Bug with useCollection and where by timeStamp.

[izakfilmalter]

Love using this lib so far. I am trying to make a paginated list of messages for a messaging app, and I think I stumbled on a bug. Because of the JSON serialization / maybe something else in useCollection, where: ['time', '<', new Data(Date.now())] doesn't return anything, but a regular get on a collection ref does.

const collectionPath = `path/to/collection`
const orderBy = ['time', 'desc']

const currentDate = new Date(Date.now())

fuego.db
  .collection(collectionPath)
  .orderBy('time', 'desc')
  .where('time', '<', currentDate)
  .get()
  .then((data) => console.log(data.empty)) // data.empty === false

const { data } = useCollection(
  collectionPath,
  {
    orderBy: ['time', 'desc'],
    where: ['time', '<', currentDate],
    listen: true,
  },
)

// data always is empty array.

[nandorojo]

Yeah...this is a known issue. At the moment, only serializable fields are valid, since this is how we store items in the cache. It seems like #46 might be a solution to this, where you could convert your date string back into a date object in the toFirestore part. It’s not merged yet, but it’s in review.

[nandorojo]

On second thought, looks like that wouldn’t exactly solve this, it’s only for data manipulation. I’d have to give some thought into this.

Does firestore allow any date string comparison for queries?

[jerber]

Yeah either you can convert the string Date back into a Date object or you can store the Dates as strings in firestore and query based on string Dates. You could also use Unix timestamps.

[izakfilmalter]

I tried a bunch of different conversations to get it working and couldn't get it. You might have better luck than I. Maybe we can change the api to something like this:

where: ['time', '<', {type: 'date', value: currentDate.unix()}]

or

where: ['time', '<', currentDate, {type: 'date'}]

[nandorojo]

Could you store your dates as Unix time stamps and then use that to query?

[izakfilmalter]

I could, but I have a lot of data with firebase.Timestamp in it. I didn't want to have to deal with drift between clients.

[nandorojo]

Got it. Something like this, like you proposed, seems like it would make sense:

where: ['time', '<', {type: 'date', value: currentDate.unix()}]

It would still check if it’s just a value, but if there’s an object, it would parse it accordingly. I’m not sure if there would be any odd edge cases to an api change like that.

[izakfilmalter]

I think this would probably be safer, you don't have to refactor code that checks where[2], and just add checks for where[3].

where: ['time', '<', currentDate, {type: 'date'}]

[nandorojo]

I think that makes sense to me. Would you like to contribute this in src/use-swr-collection?

[AlexanderArvidsson]

@nandorojo Why was this closed?
I am unable to query for dates at all.

The following code does not work, it simply returns an empty array always.

  const { data: book } = useCollection<CM.Portfolio>(
    portfolio ? `portfolios/${portfolio.id}/books` : null,
    {
      listen: true,
      where: ['date', '>', startOfYear(new Date())]
    },
    {}
  );

However, querying directly from the firestore instance works:

  const ctx = useFuegoContext();
  if (portfolio) {
    ctx.fuego.db
      .collection(`portfolios/${portfolio.id}/books`)
      .where('date', '>', startOfYear(new Date()))
      .get()
      .then((data) => console.log(data.docs.map((d) => d.data())));
  }

My dates are stored as timestamps in the database.
None of the suggested syntax you mentioned works.

Also, why do you need to change the syntax like you mentioned, if the original firestore instance queries perfectly fine with Date objects?

[nandorojo]

I can re-open if someone wants to submit a PR with this solution. Otherwise, I'll try to get to it when I have time.

[AlexanderArvidsson]

I can try implementing it, but I would like to know the reason we cannot just query with the Date object, like firestore does? Is it because of swr cache like you mentioned earlier? Could we not use some kind of serializer / deserializer for date objects when storing in cache instead?

It seems odd that the API for this package would differ from the firestore instance API.

[nandorojo]

It's because this library has to stringify the query before passing to SWR in order to have a stable key.

Maybe we could check if it's a date object before serializing and inject a type: date under the hood if it is.

[AlexanderArvidsson]

Maybe we could check if it's a date object before serializing and inject a type: date under the hood if it is.

I personally think this is the better solution. If the user specifically provides a where[3], it will not inject it. What do you think?

[nandorojo]

Yeah I'm thinking the API is, there is a where[3] field, and before stringifying, the lib checks if where[2] instanceof Date, and if so, makes where[3] into type: date.

Then, when creating the firestore ref, we check if we have type: date.

The user doesn't have to inject anything in that case, but if they happened to, it wouldn't make a difference.

[AlexanderArvidsson]

@nandorojo I have now submitted a PR (#75) which implements a proper serializer which will allow us to easily serialize any kind of object in the future. Please take a look!

[AlexanderArvidsson]

I have also noticed that you cannot query for document references. For example:

  const { data: test } = useCollection<CM.Share>(
    portfolio ? 'shares' : null,
    portfolio && {
      where: ['portfolios', 'array-contains', firestore.doc(`portfolios/${portfolio.id}`)]
    }
  );

This is most likely for the exact same reason. I will add support for serializing document references to the PR.

[binajmen]

Hello @AlexanderArvidsson,

I'm currently stuck with date filtering, and your PR would be handy! What is the current status?

Thank you.

[AlexanderArvidsson]

@binajmen I am already using my fork in production. It's up to @nandorojo to accept the pull request. There is still a few improvements that can be done in the pull request (check the comments), but I do not have time to dive into it.

[nandorojo]

@AlexanderArvidsson now that you've been using it in prod, are you confident in the PR's ability to merge?

[AlexanderArvidsson]

@nandorojo While I have been using it in prod without any problems so far, I have two concerns:

1. My application is on a small scale due to it still being in beta so it has not been tested 100%, but I have gotten no reports so far.
2. There is still the fact that it can be optimized quite a bit according to the comments that I have written in the pull request

As I said in one of my comments, we should investigate if the cache could cause problems when using multiple hooks at the same time before merging this.

I do not know how using multiple useCollection with the same snapshots will affect this. If it has problems, we could try changing the serializer from static to have one serializer per useCollection. Then, we can store the fuego inside the Serializer rather than passing it as a parameter, and we don't have to worry about other hooks interfering with the cache.

However, these concerns shouldn't be too critical regardless. If you wish to merge, then that is fine. You could create a task to look into the second concern I had.
I unfortunately do not have time to do it myself.

[samipshah100]

I am also having the same issue. Luckily found this issue quickly.

Any fix is highly appreciated!

Thanks. 😀

[ghost]

Hello, any workarounds that I can query the timestamp by range at this moment?

[345ml]

Sorry.
The repetitive behavior subsided, but I was not able to get any data.

I ran into a problem where the process kept repeating without being able to retrieve any data.

By pushing the date out of the React Component, I think this has been resolved.
I don't understand the implementation but just wanted to report it.

const date = new Date(moment().add(-7, "days").format());

const Component = () => {
  const { data } = useCollection("users", {
    where: ["createdAt", ">=", date],
  });

  return <div></div>
}

[nandorojo]

That’s because the date gets recreated on each render, so it would fetch every time. You probably want this:

const date = useRef(new Date(…)).current

and put that inside your component.

but yeah, currently this only works with a milliseconds time stamp as this issue states.

[345ml]

@nandorojo
I tried milliseconds with the following code, but the query returns an empty array.
new Date(moment().add(-7, "days").format()).getTime()

[AlexanderArvidsson]

That’s because the date gets recreated on each render, so it would fetch every time. You probably want this:

const date = useRef(new Date(…)).current

and put that inside your component.

but yeah, currently this only works with a milliseconds time stamp as this issue states.

The date is created outside the component, surely it would not be re-created every render in that case unless you use lazy loading components (which I believe still wouldn't suffer much from this problem anyway?

Scratch that, missed reading the strike throughed part that moving it outside would solve it.
Using a ref would solve it if you need a dynamic date, or a memo in some cases.