Perhaps findOne should return item | null

[morgs32]

I'm just used to prisma style return types. This might be bike shedding.
null means "not found" to me more than undefined for some reason. Please close if you find this frivolous!!

[maxnowack]

I am considering switching to returning null when something isn't found, as it is more explicit than using undefined. In JavaScript, null represents an empty value, while undefined indicates that something hasn't been defined. In the context of the findOne method, the result is empty (null) but still defined by the query selector (not undefined).

However, in most cases, whether you use null or undefined makes little difference. Since developers often have different opinions on when to use each, it is generally best practice to check for both using foo == null rather than foo === null or typeof foo === 'undefined'.

As it will still be a breaking change, I won't implement it earlier than v1.

@obedm503, I noticed your thumbs down reaction. Do you have some thoughts on this?

[obedm503]

Using undefined is perfectly fine. Either way, null and undefined are both checked by !foo as == is usually discouraged.

I'm partial to sticking with undefined whenever possible. null only makes sense when explicitly setting something to empty when it gets converted to and from json.

[mieszkosabo]

Hey @maxnowack , I noticed that when I do a full page refresh the SomeCollection.findOne({ ... }) returns undefined for few re-renders, and then eventually it returns the object.

This makes it hard to distinguish whether the object is not found and when it's not loaded yet. I tried using SomeCollection.once('received') or SomeCollection.isLoading, but for some reason they both fire up or in case of isLoading return true before the query resolves to the searched object. The thing is that I want to redirect user to different page if the object is not found, but I don't know if it's not found or not loaded yet in this case.

Maybe undefined could mean that the query didn't run yet and null could mean that nothing was found?
Or perhaps there is some other way right now to check which case is it? (I'm using React btw).

[maxnowack]

Hi @mieszkosabo, in SignalDB there is no difference between “no data” and “data is not yet loaded”. Therefore, it is not possible to return undefined if the data has not been loaded yet.
isLoading is exactly what you should use in this case. I think that what you are experiencing may be a bug. Are you using the “ReplicatedCollection” class?

[mieszkosabo]

I'm using OPFSAdapter for persistance, maverickjsReactivityAdapter for reactivity and the default memory adapter.

[maxnowack]

Yes, this is unexpected. I just found the bug. I'm fixing it right now 🙂

[maxnowack]

Fix from #895 was just released in v0.18.0 🎉

[mieszkosabo]

Thanks! I updated the lib and when I run my test I get better results, but still not perfect 🤔

BTW when I replaced the OPFFSAdapter to LocalStorageAdapter, the data is loaded right away and isLoaded() is always true, but that's probably because OPFS is async.

[mieszkosabo]

Ok @maxnowack my bad! Your fix works, it's just that useReactivity runs whenever any one thing inside it changes, so with my setup of

  useReactivity(() => {
    console.log("test", Todos.find().fetch(), Todos.isLoading());
  }, []);

I guess the isLoading changes first, so the console log is fired and then todos are returned so it fires again with both values correct.

I removed isLoading from useReactivity and just call isLoading inside the React component when the .fetch() results are changing and then isLoading has correct values 👍

I.e. something like this:

  const data = useReactivity(() => {
    return Todos.find().fetch();
  }, []);

  console.log("test", { data, isLoading: Todos.isLoading() });

Thanks again for your fix 👌