ManuelDeLeon / viewmodel-react

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

Loss of 2 way binding within conditionally rendered component #20

Open wildhart opened 7 years ago

wildhart commented 7 years ago

In the following example two way binding on the <input> element is lost:

Parent({
  sub:Meteor.subscribe('users_in_team', this.props.params.teamId),
  users:Meteor.users.find({team: this.props.params.teamId}),
  render() {
    <Users b="if: sub.ready()" teamId={this.props.params.teamId} users={this.users()} />
  }
})

Users({
  foo:'',
  render() {
    <input b="value: foo" />
  }
})

In the Users component, calling this.foo() does return the current value of the input element, but calling this.foo('bar') or this.foo.reset() updates the value of foo, but does not change the contents of the input element (although it does change the defaultValue according to React Developer Tools Chrome extension), so the binding is only one way.

Removing the b="if: ready" fixes it, so I thought it might be because the Users components is being conditionally rendered. So I tried making it conditionally rendered based on a simple flag:

Parent({
  sub:Meteor.subscribe('users_in_team', this.props.params.teamId),
  users:Meteor.users.find({team: this.props.params.teamId}),
  ready: false,
  created() {
    Meteor.setTimeout(() => this.ready(true), 3000);
  },
  render() {
    <Users b="if: ready" teamId={this.props.params.teamId} users={this.users()} />
  }
})

Users({
  foo:'',
  render() {
    <input b="value: foo" />
  }
})

But this worked fine, the input element appears after 3 seconds, and is fully 2-way bound to foo - this.foo('bar') updated the contents of the input element on screen. So it seems it is something to do with using the subscription.ready() as the condition. Weird. I'm using SSR if that matters, but I get the same problem whether or not the component is part of the pre-render (the initial route) or in a client rendered route.

If you can't recreate this easily then let me know and I'll try to crate a repro, but I'm hoping you can add this test to one of your existing projects much more easily than I can create a repro app...

ManuelDeLeon commented 7 years ago

Please make a repro.

wildhart commented 7 years ago

Repro here: https://github.com/wildhart/viewmodel-react-starter-meteor

Should be fairly self explanatory.

Sorry for the delay, I've been busy finishing my own project (for which viewmodel/react has been super helpful!)

ManuelDeLeon commented 7 years ago

I've been working on it but it's super weird. The child component does have the right value and the input box (if you inspect the HTML) also has the right value. Yet the text doesn't show inside the input box.

Oddly enough it works if I set a value on the next JS cycle:

App({
  created() {
        Meteor.subscribe('teams', () => {
            Meteor.setTimeout(() => {
                this.show(true);
            }, 0);
        });
    }, 
    show: false,
    //sub: Meteor.subscribe('teams', () => console.log("Done!")),
    render() {
        <div>
            <Child b="if: show" />
        </div>
    }
});
ManuelDeLeon commented 7 years ago

I'll be honest (for once), I have no clue. Here's the closest I've got to making it work with v1.0.4:

App({
    blah: 'A',
    sub: Meteor.subscribe('teams', () => this.blah('B')),
    render() {
        <div>
                <Child b="if: sub.ready" />
        </div>
    }
});

Of course at that point you might as well do:

App({
    showChild: false,
    sub: Meteor.subscribe('teams', () => this.showChild(true)),
    render() {
        <div>
                <Child b="if: showChild" />
        </div>
    }
});
wildhart commented 7 years ago

I wouldn't worry too much about it. The workaround is simple and I've got in the habit of using the ready callback rather than sub.ready(). Hopefully the bug is fairly readily apparent to people (I noticed a discrepancy quite quickly), and with this bug report available they should be able to find an easy workaround.

Thanks for taking the time to investigate.