fireproof-storage / fireproof

Realtime database, runs anywhere. Install Fireproof in your front-end app or edge function, and sync data via any backend.
https://fireproof.storage
Other
219 stars 16 forks source link

fix: updates React hooks to support generics properly #85

Closed thedanchez closed 5 months ago

thedanchez commented 5 months ago

This PR is in response to the issue posted here: https://github.com/fireproof-storage/fireproof/issues/81

Was an oversight on my part

Remaining TODOs

thedanchez commented 5 months ago

@jchris sounds good -- I also want to kick the tires a little bit and maybe take the opportunity to start off a new local react example app so that we can eventually deprecate the other ones and just build on the new one if that's cool with you?

thedanchez commented 5 months ago

Converting this to draft because did some tests and saw some quirks to iron out

thedanchez commented 5 months ago

@jchris so I made a slight change to the useDocument API interface for the React SDK. Ended up going down this path after seeing undesirable behavior while testing on the example UI with how our internal useCallback and useEffect kept on being re-calculated or re-ran.

The change is, instead of passing a raw anonymous object, the user will instead pass in a function that generates the document.

// Before -- raw object input
const [...] = useDocument({ text: "", date: new Date(), completed: false });

// After -- generator function input
const [...] = useDocument(() => ({ text: "", date: new Date(), completed: false }));

In order to stabilize things while keeping things as they were, the user would have to memoize the object they are passing in which I think is not something we probably want to force on them. Looking at the above example you could see how this can happen when we have something like that new Date() in the object -- it returns a new value every time the React re-render cycle kicks in. And because it changes on every render, our internal implementation things that depend on that initialDoc will trigger their recalculations which is pretty volatile.

By making it a generator function, we can memoize it internally (taking that responsibility from the user) and it'll be static despite containing something like new Date() or any other dynamic value (like some value coming from component props).

One other note here is the upcoming SolidJS SDK will go down that same path as Solid only renders once and we do not want to create "stale" versions of the initial doc object (because new Date() will have been evaluated only once in Solid land). Making it a generator function will allow us to grab a fresh new Date() value upon invocation.

Feel free to disagree with me on this change or if you think I'm off the mark here. Also, I would love for you to test things out if you're able to. Definitely would feel more comfortable knowing you were able to kick the tires on this.

I made a new React example app (under examples/react) as part of this that you can run from the root project directory using pnpm start:react.

I recommend running two shells, one which runs pnpm build:watch:react and the other will run pnpm start:react

jchris commented 5 months ago

this sounds correct, looks correct, and works great here.