Implement a computationHandler API, with a cleanup method

[CaptainN]

This implements the proposal I described in #2.

My proposal would simply add a third parameter to useTracker which would run whenever a new computation is created, and pass that reference for the user to handle. 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 before disposal - 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.

Since this is completely additive, I don't think we need to hold up the release of a beta for the main hook.

TODOs:

• Add this to the documentation.
• Maybe add some type checking in dev mode.
• Add some unit tests.

[menelike]

I like that approach, plz gimme time to this WE then we can compare your solution with mine and decide.

[CaptainN]

I added some basic tests to the repo. One of the things I'd like to create tests for is to verify some of the computation behavior. It turns out I need this API to do that!

[CaptainN]

There is an interesting problem with this API, with server-rendering support. The current very simple server implementation doesn't have any kind of computation life cycle at all. I can fake it (or partially re-implement it), but I'm not sure that's sufficient. If I try to use the regular client side implementation, errors are thrown from using the computation. I can bypass the computation for server runs, but then we don't get a computation to send to the life cycle method. I'm really not sure how to handle it for server side code.

[CaptainN]

I ended up working it so that the server still runs through the computation life cycle, but since it doesn't use a computation, simply passes in null. I think it is probably the best way to go without complicating things. This stays compatible with the current API.

What do you think?

[CaptainN]

Do you think I should test our luck and throw this at a PR?

[menelike]

@CaptainN

You can see my approach here https://github.com/risetechnologies/react-packages/blob/dev/packages/react-meteor-data/client/useTracker.client.js#L143-L171

This is WIP and has not been tested at all. Btw. some existing tests fail because ReactDOM.render(<Foo n={0} />, div); will return null with functional components if I am not wrong.

If you are interested in testing you can directly check out https://atmospherejs.com/risetechnologies/react-meteor-data

[menelike]

Please don't wonder, I have added some proper linting and up to date project structure. It also seems that tinytest might not be the best tool for this, but I am afraid that jest won't be easy to use within a meteor package.

[CaptainN]

I actually added a bunch of tests to the mian PR including porting some very old ones from the original repo, and got the remaining stalls finally worked out. Have you taken a look at those?

[CaptainN]

I'm not sure I understand what problem your handle implementation is solving. Please forgive me if I'm being thick.

[menelike]

To make it clearer, sometimes we have such a case in our codebase:

  state = { offline: false };

  componentDidMount() {
    this.computation = Tracker.nonreactive(() =>
      Tracker.autorun((computation) => {
        const { connected, status } = Meteor.status();

        if (status !== 'connecting') {
          this.setState({ offline: !connected });
          if (connected) computation.stop();
        }
      })
    );
  }

  componentWillUnmount() {
    if (this.computation) {
      this.computation.stop();
      this.computation = null;
    }
  }

I could use the useControlledTracker to stop the tracker completely, either from the reactive function or from react. The reason why I like to be able to control this from react is because of reusability. If I would put that kind of logic into the reactive function its behavior becomes static. If I am able to control the behavior from react, I'd be able to use it depending on my use case (I have other places where I have the same logic, but the computation should not stop afterward).

[menelike]

I actually added a bunch of tests to the mian PR including porting some very old ones from the original repo, and got the remaining stalls finally worked out. Have you taken a look at those?

I used them and had a typo in there, d'oh, thanks for setting them up

[CaptainN]

I'm still not sure why you need to stop the computation when you are offline, but if we pass the computation (or your computation state abstraction) to the reactive function, would that give you what you need?

const [offline, setOffline] = useState(false);
useTracker((computation) => {
  const { connected, status } = Meteor.status();
  if (status !== 'connecting') {
    setOffline(!connected);
    if (connected) computation.stop();
  }
}, []);

For re-usability, one of the nice things about hooks is how you can simply create a set of predefined hooks, and use them everywhere. If you wrap the above inside a hook called useStatusAndStop or something, you could reuse it, and if you leave out the computation.stop() you could have another hook, useStatus.

// or a configurable version
const useStatus = (doStop = true) => {
  const [offline, setOffline] = useState(false);
  return useTracker((computation) => {
    const { connected, status } = Meteor.status();
    if (status !== 'connecting') {
      setOffline(!connected);
      if (doStop && connected) computation.stop();
    }
  }, []);
};

I actually used to do that even with the old withTracker too - I called it a connector pattern (a connector is like a generic container prefab).