vigetlabs/microcosm

A Presenter's "update" method is not run when the model changes

Closed this issue ยท 8 comments

What kind of issue is this? (check at least one)

  • Report a bug
  • Feature Request
  • Question
  • Discussion
  • Documentation
  • Other, please describe:

What version of Microcosm is this for

I ran the example code in 12.9.0-beta4 and 12.8.0 with the same results.

Steps to Reproduce/Example (if applicable)

I expected that a Presenter's update method to be run whenever the Presenter's model changes, but this doesn't appear to be the case. The documentation states:

update is always executed after the latest model has been calculated.

Here is a a bit of example code illustrating the behavior I'm experiencing:

import React from 'react';
import ReactDOM from 'react-dom';

import Microcosm from 'microcosm';
import Presenter from 'microcosm/addons/presenter'

//
// SETUP THE TEST
//

// Action
function increment() {
	return true;
}

// Domain
class CounterDomain {
	getInitialState() {
		return 0;
	}

	register() {
		return {
			[increment]: this.handleIncrement
		};
	}

	handleIncrement(state) {
		return state + 1;
	}
}

// Presenter
class Test extends Presenter {
	setup(repo) {
		repo.addDomain('counter', CounterDomain);
	}

	getModel() {
		return {
			counter: state => state.counter
		};
	}

	ready(repo, props, state) {
		console.log('ready', this.model.counter);
	}

	update(repo, nextProps, nextState) {
		console.log('update', this.model.counter);
	}

	render() {
		console.log('render', this.model.counter);

		return (
			<div>
				Counter is at: {this.model.counter}
				<hr/>
				<a href="" onClick={this.handleClick.bind(this)}>Increment</a>
			</div>
		);
	}

	handleClick(evt) {
		evt.preventDefault();
		this.repo.push(increment);
	}
}

//
// RENDER THE TEST
//

ReactDOM.render(
	<Presenter repo={new Microcosm()}><Test/></Presenter>,
	document.getElementById('root')
);

Here's a screenshot after clicking the "Increment" link six times (notice we never see an "update" string logged):

screen shot 2017-07-24 at 2 29 25 am

Is update supposed to be run in this case? If not, what is the recommended method for reacting to model updates in a Presenter?

Hey @djmccormick, thanks for the great write up!

update does not get called when the model updates. This is to avoid accidental infinite loops (the model changes, you push an action, the model changes, you push an action, and so on).

The documentation intended to say that the latest model is available when update is called.

Presently, we don't have a lifecycle hook for when the model updates, but we could. What do you think of something like:

modelWillUpdate (repo, nextModel) {
  // do a check, this.model is the last model
}

Totally open to a different name. And we'll fix the documentation to be clearer.

@nhunzaker would there again be a risk of triggering an infinite loop if modelWillUpdate pushed an action?

๐Ÿ‘ let's make that super clear in the docs if we end up implementing something like this. How do we feel about modelWillUpdate and/or modelDidUpdate? @djmccormick did you have a specific use case in mind?

Thanks for bringing this to our attention @djmccormick!

That line in the docs is misleading, sorry about that - here's an amended version #375

I like the idea of escape hatches for hooking into model update lifecycles and the symmetry of modelWillUpdate/modelDidUpdate with componentWillUpdate/componentDidUpdate. I too am curious about specific use cases. This has not come up as a need yet in any of the projects I've used Microcosm for.

In my specific case, I am using React Router v4 + Firebase with Microcosm. Firebase has the ability to listen for user authentication changes, leading me to believe that a user could get logged in or out unexpectedly. If a user is sitting on the login screen and they suddenly become authenticated, I'd like to send them to their dashboard. Similarly, if they're viewing their dashboard and suddenly become unauthenticated, I'd like to send them back to the login screen.

I am listening for this auth change in a global way through non-component or code. I send an appropriate action to the repo, and my data gets cleaned up, but I don't have history from React Router easily accessible in order to tell where I am and change it. There are a few ways around this, I suppose. I could move my auth change detection into high level Presenter code and work off the action.onDone. Or, I could leave my detection as it is and keep history in the repo and do the redirection in an effect. What I was attempting to resort to last night at 2:30AM was to detect the repo state change in component code and react to it there. In retrospect, that's probably the most brittle way about it. I'm not sure adding model change detection lifecycle events is even a good idea, but I'll leave that judgement in your capable hands.

Do you have a suggestion of your own or a preference around the approaches mentioned above?

Thanks for your feedback on my issue, and thanks to @solomonhawk for the PR clarifying things in the docs. And thanks, of course, for creating and maintaining the best data and state management tool I've ever used.

+1 to modelWillUpdate/modelDidUpdate.

I recently had an issue trying to hook into model data changes using .update and found that a Presenter's .update wasn't getting called because Presenter .teardown was getting called first.

I was thinking โ˜๏ธ could be a separate issue, but adding the more explicit hooks for data changes seems like it would make this irrelevant.

@djmccormick I've added a modelDidUpdate method in microcosm@12.13.0-rc:
http://code.viget.com/microcosm/api/presenter.html#modelwillupdaterepo-nextmodel-changeset

import { getPlanet } from '../actions/planets'
import { find } from 'lodash'

class Planet extends Presenter {
  getModel(props) {
    let { id } = props.location.query

    return {
      id: id,
      planet: state => find(state.planets, { id })
    }
  }
  modelWillUpdate(repo, nextModel, changeset) {
    if ('id' in changeset)
      repo.push(getPlanet, changeset.id)
    }
  }
  // ...
}

I am hesitant about the ability to produce infinite loops, but it patches a hole I think we've had in the presenter API for a long time.

Please let me know what you think!