ManuelDeLeon / viewmodel-react

Create your React components with view models.
MIT License
24 stars 3 forks source link

Question: Re-execution of function in repeat binding? #7

Closed fvpDev closed 8 years ago

fvpDev commented 8 years ago

I have a menu being rendered from a prop mainMenu like so:

<div id="mainMenu" class="ui horizontal link list">
  <a b="repeat: mainMenu, href: '/' + repeatObject.id, text: repeatObject.name, class: { active: isActive(repeatObject.regex) }" class="item"></a>
</div>

with isActive(regex) { return ActiveRoute.path(new RegExp(regex)) }

When I click on links to change routes, the route changes, however it seems that the active class does not change for the links as if isActive() doesn't get re-evaluated. Am I doing something wrong? Is there a way to bind the function rather than the result of the function?

Also, I'm fairly new to React so I was wondering: in what cases is a key required on a repeat binding and why is it needed?

ManuelDeLeon commented 8 years ago

Seems like ActiveRoute.path isn't reactive and ViewModel has no way to pick up the changes.

The key property is equivalent to the _id property in blaze. It tells React how to recognize elements in a list/loop in case they're moved around (for example if an element is deleted).

fvpDev commented 8 years ago

Ah, gotcha (about key). So as a solution I could make ActiveRoute.path() into a ReactiveVar somehow, yeah?

ManuelDeLeon commented 8 years ago

I would use a share and update it via whatever mechanism ActiveRoute notifies you that the route changes.

ManuelDeLeon commented 8 years ago

Another option is that if main menu owns the routes (aka is always visible) then you can have a path property in that view model and access it directly in the repeat element. You would still need to update this property when ActiveRoute changes though.

fvpDev commented 8 years ago

Right...it's interesting because according to https://github.com/zimme/meteor-active-route/blob/master/lib/activeroute.coffee#L100-L102 ActiveRoute uses FlowRouter.watchPathChange() which is reactive before getting FlowRouter.current() which is non-reactive. See https://github.com/kadirahq/flow-router#api

ManuelDeLeon commented 8 years ago

Are you using Meteor's Tracker?

Before doing anything else:

import { Tracker } from 'meteor/tracker';
import ViewModel from 'viewmodel-react';
// Use Meteor's dependency management
ViewModel.Tracker = Tracker; 
fvpDev commented 8 years ago

Uh huh, that's the first thing that gets loaded... Found this https://github.com/zimme/meteor-active-route/issues/45

ManuelDeLeon commented 8 years ago

It has little to do with React. If ActiveRoute triggers a .depend() and .change() in a Tracker.Dependency object when you call ActiveRoute.path then active should change. I think it might be a bug with ViewModel and the way it behaves with the repeat binding. I'll check it out when I get a chance.

fvpDev commented 8 years ago

Alright, sounds good, looking forward to it! :D

ManuelDeLeon commented 8 years ago

Well, I can confirm that reactivity is working inside repeated elements. For example the following works fine:

App({
  color: 'red',
  colors: [
    {
      color() {
        return ViewModel.findOne('App').color();
      }
    }
  ],
  render(){
    <div>
      Color: <input b="value: color" /> <br/>
      <div b="repeat: colors, text: repeatObject.color" />
    </div>
  }
});
fvpDev commented 8 years ago

Ok, I just confirmed for myself through autorun() { console.log(this.mainMenu()) } that mainMenu updates reactively upon route change, and that the problem is not with ActiveRoute...Gonna try to make a repro for you

ManuelDeLeon commented 8 years ago

I don't think mainMenu is the problem. Does ActiveRoute.path(new RegExp(regex)) change inside an autorun?

fvpDev commented 8 years ago

Yup, and updates mainMenu, which btw is defined like this:

isActive(regex) { return ActiveRoute.path(new RegExp(regex)) },
mainMenu() {
  return this.sitemap()
    .filter((route) => _.has(route, 'i'))
    .map((route) => { return { id: route.id, name: route.name, active: this.isActive(route.regex), updated: new Date().getSeconds() } })
},
autorun() { let m = this.mainMenu(); console.log('mainMenu: ', m[0].active, m[1].active, m[2].active, m[3].active) },

I couldn't recreate this in React repro (w/o meteor) but I think the problem may be the _.has; checking now with .hasOwnProperty instead.

Update: ^didn't help... Did you manage to download Meteor the other night/do you mind checking out my project (I haven't updated it yet)?

fvpDev commented 8 years ago

Interesting...just tried

<a class="item" href={'/' + this.mainMenu()[0].id}>{this.mainMenu()[0].name + ': ' + this.mainMenu()[0].active}</a>
<a class="item" href={'/' + this.mainMenu()[1].id}>{this.mainMenu()[1].name + ': ' + this.mainMenu()[1].active}</a>
<a class="item" href={'/' + this.mainMenu()[2].id}>{this.mainMenu()[2].name + ': ' + this.mainMenu()[2].active}</a>
<a class="item" href={'/' + this.mainMenu()[3].id}>{this.mainMenu()[3].name + ': ' + this.mainMenu()[3].active}</a>

and it looks like for autorun mainMenu is changing but in render() nothing is changing...like it's not 're-rendering', like render() is not reactive.

ManuelDeLeon commented 8 years ago

I'm kinda lost. Please make a repro.

fvpDev commented 8 years ago

Can you use meteor + react or no? I can't seem to recreate in just React

ManuelDeLeon commented 8 years ago

Sure, make a version that runs with Meteor.

fvpDev commented 8 years ago

Ok, going to try one more thing then I'll repro

fvpDev commented 8 years ago

https://github.com/fvpDev/vm-meteor-react-repros

ManuelDeLeon commented 8 years ago

The problem is that you're using a really old version of viewmodel-react. Update to v 0.6.3 ;)

fvpDev commented 8 years ago

Lol, when should I update? image

ManuelDeLeon commented 8 years ago

Now =)

On Aug 22, 2016 8:31 PM, "Fedor Parfenov" notifications@github.com wrote:

Lol, when should I update?

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/ManuelDeLeon/viewmodel-react/issues/7#issuecomment-241609730, or mute the thread https://github.com/notifications/unsubscribe-auth/AED31kGoMua38D--vBhQqS50aAskspLxks5qiltrgaJpZM4JpVkO .

fvpDev commented 8 years ago

Well dopeski, thanks for being awesome! I'll update tmrw :smile: