Actually it doesn’t, if the helper has different dependencies depending on the argument, then the first call will not properly invalidate. I can send you a repro if you want to see it.
And what’s bad about that? I’m super confused about this statement. How could you evaluate a helper that takes an argument without knowing what the argument is?
I think maybe a simple approach is that if people want to improve the performance of their components (i.e. avoid “re-rendering too often” problems), they can use shouldComponentUpdate or a computed field.
Personally I’m an advocate for wrapper components where the “smart” component uses the reactive data sources (and doesn’t really do anything but fetch data), and the child “pure” component simply renders based off that. Then you can use shouldComponentUpdate to your hearts content in the child if you want to get performant.
We’ll have to agree to disagree on this one. Event handlers reaching into child templates has got to be the biggest problem with the current Blaze API in my opinion. Making it difficult to pass event handlers into children (a much better way of communicating between components) is another (related) one.
I think React got the “data goes in one direction through the hierarchy only, apart from callbacks” thing exactly right. It’s a little restrictive, but sometimes constraints actually help you in the long run.
callReactiveHelperMethod() {
return Session.get('foo');
},
render() {
if(false) { //obviously the value of false is a dynamic variable
let foo = this.callReactiveHelperMethod();
}
//return whatever
That render call won’t know to reactively re-render.
well, having a parser automatically make the attachments for you is identical to manually doing it. The problem is what Blaze is missing that doesn’t let you make full use of the pattern (and thats where you end up getting yourself into trouble–not just purely by having jQuery style event selectors). What blaze is missing is access to the state of the parent component that defines the event handler from helpers called in the child component.
So what Blaze 1 has is essentially event handler dynamic inheritance from parent component to child component, right. I’m proposing that we also have helper inheritance to match up with it. AND then in both cases fix the issue of what Template.instance() or state refer to: it doesn’t refer to the state of the template/component that the helper/handler runs in, but the dynamic parent one it was defined in.
The net result is the exact same benefits you get in React: being guided into the “pit of successful state management success,” but with more ease/automation, less friction. The workflow becomes:
define the helpers and event handlers on the component that manages the state
DON’T WORRY ABOUT PASSING DOWN EITHER THE EVENT HANDLERS THAT ACTS ON THE STATE NOR THE HELPERS/DATA THAT PRESENTS THE STATE
simply call the corresponding helper methods in child components.
That’s also in the jsfiddle–well basic proof that access to the parent works reliably (we’d need our transpiler to add references to the parent for us if we cant figure out how to do it automatically in some React mixin or override). You can pass down a parent by reference. When a parent changes, you’re re-rendered anyway, you now have access to that parent and the new returns of its helpers. All automatically. Getting it to play nice with the pure render mixin we’ll save for another day, but can likely have a solution too–that is if it doesn’t just work like regular components. The point is that if the whole branch of the tree is re-rendered and you’re a child component in it you can just access state on a parent by reference, i.e. a reference to the parent, and then utilize a helper method on the parent, which then retrieves data from its own state, bringing it into the child component. We haven’t violated the laws of “unidirectionality”, only “immutability” --ever so slightly. I’m personally not opposed to breaking rules if it works–it’s how the web evolved to what it is and continues to evolve: the standardization of hacks that achieved majorly in-demand features when nothing else could. Tracker itself could be considered one such hack. Same with virtual dom. Each wasn’t the standard way of going about things prior to its existence. If we can achieve this in a 100% reliable fashion, we have the works for a better interface than React proper.
@sashko@tmeasday render method based reactivity is done and the package is updated. The example app is also updated to show how these problems dont exist anymore:
git clone https://github.com/ultimatejs/tracker-react-todos-example.git
cd tracker-react-todos-example
meteor
The example repo’s readme highlights how tracker-based reactivity plays nice with React’s state-based reactivity, thereby being the key mechanism that solves the issues discussed today minus the Tracker.fush() one:
@tmeasday’s helper methods being called with different arguments in React-triggered re-renders - no longer applicable
@tmeasday’s potentially incorrect methods becoming reactive - no longer applicable
@faceyspacey’s different code paths possibly not triggering certain helpers and render method not being made into a new autorun on React-triggered updates - was already solved if moved old reactive binding code just to render method
…It just keeps on getting easier and easier–the helper-based version was like half as much code as the original getMeteorData version, and this version is like half the code as the helper-based one. Now that the autorun re-binding is happening every time render methods are called, even if triggered by React renders (rather than only on componentWillMount like before) we are able to overlap perfectly with React-triggered state-based reactivity. I do wonder what the performance penalty is–if any–for constantly recreating our autoruns on every single component’s re-render.
As for the Tracker.flush() issue, it seems we first need a mechanism to synchronously flush React. Then we call both Tracker’s and React’s flush back to back:
Tracker.flush(); //trigger forced updates
React.flush(); //synchronously ensure all components are re-rendered
//execute code you usually would (with expectation that DOM is up to date)
Am I missing something?
The closest thing I can find available in scope is: React.addons.batchedUpdates(). But it seems we want ReactUpdates.flushBatchedUpdates() which isn’t exported for us. Theoretically we aren’t using jQuery or velocity or whatever for animations anymore, and are instead using the React animation addon or one of the better community animation packages; and instead of Tracker.afterFlush() we are using lifecycle hooks like componentDidUpdate–so we have no need for Tracker.flush/afterFlush() in order to manually manipulate the DOM. And of course we use things like refs to access form values in the DOM. Tracker.flush/afterFlush() may turn out to be a non-issue.
Very pro-active. Thank you. It seems a great way to go. Reading your piece a few times, to get the state of mind we need to get back into full Meteor trust…
That was long read
Sometimes I think that observing DDP and maintaining 1 big immutable Atom kinda redux style replacing minimongo would solve many recomputing issues.
But who would sacrifice time to write that…
React could connect kinda natively as normaly in Redux and there could be some easier hacks to simulate normal cursor invalidation events.
But that is just my feeling.
Edit: And it is not minimongo, but whole collection handling in mongo package when I am now fast looking into source. Still it all use/call LocalCollection which is defined in minimongo. So probably replace it with other LocalCollection object which will be internally using redux store. That would be fun.
I don’t. …tho, I’m curious to what the issue was. React-triggered re-renders were re-creating the autorun based on the new arguments. Maybe I was missing something.
Then if you did Session.set('a', 'x') it would not re-render, as the autorun that depended on Session.get('a') would have been destroyed in setting up the autorun the second time (to depend on Session.get('b')).
I guess the problem is that even if that existed, we can’t rely on users (e.g. package authors like in the case I was quoting in the bug, where it was FlowRouter calling Tracker.flush()) calling React.flush() – I mean why would they when writing a package that’s not relevant to React?
I think the real answer here is that Tracker.flush() is a bad idea unfortunately, as @dgreensp concluded. All we can do now is deprecate it in future versions of Meteor.
The vision for all this would likely involve overwriting Tracker.flush/afterFlush to call the React implementations if React is detected as the renderer.
I’ve just discovered this package this morning. I was extremely confused at first since the description in the initial (and veeeeeery long) post concerning functions starting with underscores etc didn’t seem to match the code. However, having now read these comments I see that it’s since evolved considerably.
In any case, the current implementation is simply excellent. This is a much nicer solution than the official mixin and it’s really fantastic to have such natural Meteor-based reactively so tightly integrated into a Blaze component. Well done.
Ya, this is a very nice step in the right direction. ReactMeteorData required too much manual connecting than normal Blaze and Meteor that we are all used to. Good stuff!
