Hooks cleanup

[CaptainN]

This PR adds a cleanup method to the useTracker hook. This cleanup function can be used to allow for easier cleanups after a hook has been used, and discarded.

Here are two examples - a useSubscription hook, and a useSession hook, implemented both with and without. Without first:

// without cleanup, I have to use useEffect, which is cumbersome:

export const useSubscription = (name, ...rest) => {
  let handle
  const isReady = useTracker(
    () => {
      handle = Meteor.subscribe(name, ...rest)
      return handle.ready()
    },
    [name, ...rest]
  )
  useEffect(() => () => handle.stop(), [handle]) // cumbersome
  return isReady
}

export const useSession = (name, defaultValue, cleanup) => {
  const value = useTracker(() => {
    Session.setDefault(name, defaultValue)
    return Session.get(name)
  }, [name, defaultValue])
  // cumbersome
  const cleanupRef = useRef()
  cleanupRef.current = cleanup
  useEffect(() => {
    if (cleanupRef.current) {
      return cleanupRef.current
    }
  }, []) // cumbersome
  return [
    value,
    (val) => Session.set(name, val)
  ]
}

With cleanup next:

export const useSubscription = (name, ...rest) => {
  let handle
  const isReady = useTracker(
    () => {
      handle = Meteor.subscribe(name, ...rest)
      return handle.ready()
    },
    [name, ...rest],
    () => { handle.stop() }
  )
  return isReady
}

export const useSession = (name, defaultValue, cleanup) => {
  const value = useTracker(() => {
    Session.setDefault(name, defaultValue)
    return Session.get(name)
  }, [name, defaultValue], cleanup)
  return [
    value,
    (val) => Session.set(name, val)
  ]
}

// the useSession use case (cleaning up Meteor Session values is ... hacky)
const [myVal, setMyVal] = useSession('myVal', '', () => delete Session.keys['myVal'])

[menelike]

I was thinking about something similar initially but skipped that as I thought it might be overengineered. I had a use case where I had to stop the reactive computation after the first run.

I had two APIs in mind, the first is a "pause" flag like:

const foo = useTracker(() => {}, deps, pause)

where pause is either true or false. What I don't like about this is that you have to switch pause in your own code which usually means you need useState just for that, including re-rendering just for pausing.

The other version would look like:

const [foo, computation] = useTracker(() => {}, deps)

which is very similar to you your version, but IMHO is more consistent with hooks or at least to useState API wise. In contrast to the first version, you need to be careful when deps change as you need to stop the restarted computation again afterwards.

Maybe an even simpler approach could be something like:

const foo = useTracker(() => {}, deps, once)

where once is taken into account only on mount and the computation is forever stopped afterwards (in this case passing deps obviously doesn't make sense).

Whatever we do, this API needs to work well for the future as changing the behaviour leads to breaking changes, so we should take our time and grind this as long as it needs to.

For instance, what if I need to restart the computation after a while, changing deps just for this reason is cumbersome as well.

Final words:

I absolutely like the idea of pausing/controlling computations, in my project I sometimes have to keep component trees mounted while navigating to other pages (in order to keep the state of the dom tree, scroll position etc. and especially since mounting is more expensive than keeping), but since the component tree is in the background pausing the complete tree (ideally with usecontext) would help with performance issues.

[CaptainN]

Yeah, in playing around, I actually had a hard time working with this cleanup method in cases where I was writing a new hook around useTracker, and the props might change. cleanup would be easy if it only ever needed to run un unmount, but it gets tricky with retaining references when deps changes. I'd say to table this for now.

I do like the tuple approach for allowing the user to access the computation.

[CaptainN]

I have an idea! What if we just pass the computation to the user's reactive func?

// preserve `this` and pass the current computation to the reactiveFn
const data = reactiveFn.call(this, c);
if (Meteor.isDevelopment) checkCursor(data);
refs.trackerData = data;

This wasn't done in withTracker because it provides props instead, but we currently don't pass anything to the reactiveFn - might as well pass the computation. We can also preserve the this context. Some examples out there on the toobs use code which includes stuff like this.subscribe('subname') - it's probably not a big deal, but it's low hanging fruit.

Edit: Alternatively, or additionally, we could pass the computation to a cleanup function. Maybe we should discuss this particular change on the main PR thread so there are more eyeballs on it.

[menelike]

@CaptainN Indeed we should talk with the other folks about this.

I have an idea! What if we just pass the computation to the user's reactive func?

I absolutely like this idea because of the clean API including enough space for all quirks developers need. The only thing which might be missing then is how to restart the computation? Maybe passing a third arg which triggers a recreation of the computation might work...but 2 args? not sure.

Since MDG is somehow slow with releasing, I am actually thinking to create a permanent fork and to release a custom package. I'd like to add you as a maintainer then, wdyt?

[CaptainN]

I'd be down with maintaining our own. I have a project which relies on this heavily, and thought about releasing a version of this hook now, so that I can release my own tool, then updating it to simply wrap the main release if MDG ever get around to releasing it. I'd really prefer to have this in the main package, since it does fix up the issue with the use of deprecated lifecycle methods, but I guess at some point that will become enough of an emergency to get MDG to do something about it.

[menelike]

@CaptainN

What do you think about this (top-down) approach:

const [value, handle] = useTracker(() => v, []);

And handle has an API like

paused: boolean;
pause = () => void: (does nothing when paused === true);
resume = () => void; (does nothing when paused === false, otherwise *always* restarts the computation with a re-render as a result);

We may also add the computation as an additonal property.

Also, a context might be used internally to pause subtrees (still not sure what the most elegant solution should look like)

[CaptainN]

I wonder what problem we are trying to solve. Originally, I just wanted a way to clean up a variable on unmount. For example, here is a useReactiveDict hook I worked out:

import { useRef, useEffect } from 'react'
import { useTracker } from 'meteor/react-meteor-data'
import { ReactiveDict } from 'meteor/reactive-dict'

// Manages and syncs the lifecycle of the ReactiveDict to the 
// React lifecycle, but does not provide reactivity to individual
// variables. For that, see `useReactiveDictVar`.
export const useReactiveDict = (name, defaultValues = {}) => {
  const { current: refs } = useRef({})
  if (name !== refs.name) {
    if (refs.dict) refs.dict.destroy()
    refs.dict = new ReactiveDict(name, defaultValues)
    refs.name = name
  }
  useEffect(() => () => { refs.dict.destroy() }, [])
  return refs.dict
}

// Takes a ReactiveDict instance, and wires up individual keys
// for react reactivity.
export const useReactiveDictVar = (dict, key, defaultValue) => {
  dict.setDefault(key, defaultValue)
  const value = useTracker(() => dict.get(key), [dict, key])
  return [value, (val) => dict.set(key, val)]
}

// Combines the above to create an API compatible alternative to
// React's `useState`, which survives hot code push (HCP)!
export const useMeteorState = (name, defaultValue) => {
  const dict = useReactiveDict(name)
  return useReactiveDictVar(dict, 'value', defaultValue)
}

// Essentially, a global version of `useMeteorState`.
let session
export const useSession = (key, defaultValue) => {
  if (!session) {
    session = new ReactiveDict('session', {})
  }
  return useReactiveDictVar(session, key, defaultValue)
}

or to cleanup after a subscription, though I'm pretty unclear about how or why you'd need to do that (or how the Meteor system actually manages that).

For this usage, I probably don't really need access to the computation at all (or to cleanup functionality - since I can use useEffect). But I do wonder about cases you've mentioned, and I'm not sure I understand the use case for pause/resume/etc.

Without really understanding that, I wonder what's required for making this work well. Do we need to maintain a reference for the user, or is just passing them a reference to whatever the current computation enough? We can probably pass them a reference to the "last" computation before it changes (in a cleanup hook).

[CaptainN]

Part of the problem with this is that the computation can change for various reasons (mostly in response to deps changes, or if there are no deps). I guess your solution would be to abstract that out, and I assume if the computation is rebuilt after being paused, it would pause the new one. In that way the state would be handled in the hook, and imposed on the computation? That could be a nice way to handle it, even with the below solution.

But I wonder if it would be simpler to add an additional method which allows the user to handle changes to the computation themselves.

const value = useTracker(
  () => v,
  []
  (c) => { // maybe this could even take (nextComputatoin, prevComputation)
    // Only called when there is a new computation - always synchronously after
    // the reactiveFn on first run, after a deps change or on initialization.
    c.onStop(() => { // do stuff })
  }
);

That's really spaghetti against a wall, but the reason I prefer this approach is that handling the computation is to me, conceptually more like useEffect than a set method from useState. I also prefer to keep the return value of userTracker clean.

I can be convinced, I'm just not sure I understand the use cases well enough.

[menelike]

I assume if the computation is rebuilt after being paused, it would pause the new one.

Yeah, I think I understand your need, you need to call the reactive function once if deps have changed. My proposal would stay paused even if the deps change over time. I need to think this through, that's a valid point.

[CaptainN]

Hi @menelike,

I ended up shipping a version of this in my latest package - a hooks based wrapper for ReactiveDict to make an easy way to store state which can survive HCP, and another hook which works like Meteor's old session package, but also works server side (it requires a provider and uses the context API).

I also figured out how to do some unit tests in there, which is pretty neat.

BTW, do you have another way to communicate? I posted here because I don't have your contact info. Are you on the forums?

[CaptainN]

My proposal would simply add a third parameter to useTracker which would run whenever a new computation is created, and pass that reference to the user. We can also have that return a function which would run against the previous instance:

const value = useTracker(
  () => v, // reactiveFn
  [v] // deps
  (c) => {
    // Called when there is a new computation.
    return () => {
      // runs right before a new computation is created, or when the entire hook is disposed
    }
  }
);

To me, exposing the reference to the computation this way this is the most flexible way to do this, without creating a ton of intermediary code, and this withEffect modeled method should be clean and familiar to hooks users.