How to handle reactive properties - discussion

[Urigo]

Hello, In version 1.3 we introduced the helpers syntax which can take 2 types of values:

1. Functions - The function return value will be binded to scope and re-run reactively
2. Primitives - A primitive we be binded to scope and will trigger a reactive change every time it will change (reactive var). It's purpose is the same as getReactively but it is implemented differently.

There is one issue with the current solution where when the primitive will change in a nested value, it won't fire a reactive change event (https://github.com/Urigo/angular-meteor/issues/939)

But that issue made me think about the current API and if it is the best solution.

Here are a few options for that API and I would love to hear your thoughts about that:

1. Stay with the current solution, meaning that helpers understands the difference on the value that is sent to it and according to that defines the value to be a reactive property (and fix the issue)
2. Separate. helpers to be just for functions and reactiveProps would be for primitives.
3. Continue using getReactively like we do today meaning, we define regular Angular variable as usually and when we want to use them reactively we surround them with a getReactively function. we can also rename it to track or something nicer
4. Create a new way to declare reactive objects in meteor. Since reactiveVar only allows editing the whole object, that would mean we won't have reactivity when we change a property of that object. Instead we can create a new way to declare a reactive object in the following way:

$scope.myVar = ReactiveVar.from({
    a : 1,
    b : {
        c : ReactiveVar.from(4)
    }
});

This should allow us to say explicitly which properties we want to be reactive in the reactive var that we are creating. That way we also won't need to call any helper function on the scope, just declare a new property, or a variable if we are not using scope at all.

5. Other options you might think of

There is also an important mention about the implementation. In the helpers/reactiveProps solutions we need to watch the nested values but also give you the option not to do a nested watch as sometimes it can be bad for performance. With getReactively we use Angular's watch with the same Angular API - the default is a regular Angular $watch but we can send a second parameter as a flag that if is true it will watch the value in a deep way. If you think that helpers or reactiveProps are the way to go, please also suggest how a deep would look in this API. We will add the new API in 1.3.1 but leave the current implementation (with a nested fix) till 1.4 so we won't introduce more breaking changes. Waiting to hear your suggestions

[Deadly0]

@Urigo I like new API, especially that fact that we don't use collections wrappers anymore. In my app, where alot of data loading on one page, i see significant performance increase.

I guess we should stay with helpers and add deep reactive to non-function properties. IMHO, the main use of helpers is to receiving data from collection. If you use primitives in helpers it's shouldn't be so huge to cause performance issues. Also there will be super nice if we will have an option to disable deep reactivity for property in helpers. I don't familiar with angular-meteor sources but may suggest:

$scope.helpers({
    deepReactive: {/* ... */} // Property deep reactivity by defaut.
    reactive: $scope.ReactiveVar({/* ... */}, false) // false disables deep reactivity.
});

I think a lot of developers agrees than new API looks really cool, but it needs some improvements to use it in production.

Thank you, Urigo!

[shantanubhadoria]

I was using the old syntax i.e getReactively in my code and I moved over to helpers syntax only to realise midway that it doesn't do a deep watch.

To be honest I prefer the front-end data binding to be done by angularJs code as much as possible, because thats what its good at, so I preferred the old syntax. That way everything worked in a angularjs way. If we wanted blaze syntax we would have just used blaze templates. So I would suggest to look at the target users who prefer to use angularjs with meteor's reactivity. I don't think they like the idea of the syntax being more Meteor-like.

Do note that when we do some sort of join on the frontend where results of one subscription determine the query for the second subscription(I was doing this with my old code), functions might break again if the return value of first subscription used as subscription parameters for second subscription are not treated 'deeply' reactive.

HTH, -Shantanu

[sebakerckhof]

At first I was with @shantanubhadoria on this. Now that I've converted a part of my application, I found cases where I prefer the old API and cases where the new API is more useful. In most cases both are pretty equal and it comes down to a matter of taste.

About the 4 options you propose:

1. If this can be done in a performant way, but I can't think of any way without deep watching. The one solution I can think of is letting the user specify himself which specific properties are to be watched, which brings us to solution 3.
2. What do we win by this separation? I fail to see how this separation helps to solve the problem compared to a simple _.isObject / _.isFunction check in helpers.
3. I'd opt for this solution. This gives the most control to the user. Additional advantage is that you get the error-cascading functionality of the angular lexer. E.g. In angular I can watch object.property.subproperty with object being undefined at certain points in time (usually when data is not yet available), thus without me having to say if(object && object.property){...}
4. This could be a good solution if it would be automated. E.g. If me, as an end user, doesn't need to make my objects similar to your code example, but if a reactive object could automatically make every property reactive, also if new sub properties are added. So in a way I can just use a fully reactive object like a normal object (should be possible with getters and setters?).

However, since usually you only have a very limited set of properties on an object you want to react on in other helpers, the additional few watches in the digest cycle will probably have less performance impact then having to transform traditional objects to fully reactive ones?

5 . Immutables. Ensure that all reactiveProps are immutables and therefore reduce a deep watch to a single watch. But this would require major changes, doesn't fit angular 1 very well and comes with its own set of problems. I just mention it for completeness

I should give it more thought, but getReactively seems the best option to me.

Anyway, thanks for opening up the discussion about future changes to more users.

[Urigo]

Thank you so much for the valuable feedback!

I've also have offline and online talks with @yyx990803 @stubailo @netanelgilad @dotansimha

Looks like the direction for 1.3.1 will be what @sebakerckhof has suggested.

getReactively seems like the cleanest and less intrusive solution of them all. It means that defining properties in Angular will stay exactly the same and only when we write Meteor specific code (helpers) we will use something to connect the two technologies. It also solves the nested changes issue.

As I don't want to introduce more breaking changes in that version, we will solve the nested problem on helpers with watch but will add a deprecation warning when using objects and primitives on helpers that directs the developer to use getReactively instead.

We will also update the blog post, docs and tutorials to use getReactively where needed.

[DmitryEfimenko]

TLTR: I think the new api is great! ... with just the exception that properties of objects placed in the .helpers() are not reactive.

Details: For me it all really boils down to the following example written in TypeScript:

class SearchCtrl extends ReactiveComponent {
    parties: IParty[];

    /* all props of this object (searchCriteria) should be reactive
     * we need to declare it here so that typescript has 
     * knowledge of this object in the context of "this"
     * we'll also reference this object in the helpers to actually make it reactive
     */
    searchCriteria = {
        name: '',
        date: undefined,
        page: 1
    };

    static $inject = ['$scope', '$reactive'];
    constructor(
        private $scope: angular.meteor.IScope,
        $reactive: any
    ) { 
        super();
        $reactive(this).attach($scope);

        // returns object that contains selector and options for the Mongo.find() function
        // follows principles described in [this article](https://www.discovermeteor.com/blog/query-constructors/)
        let params = queryConstructor('getParties', this.searchCriteria);

        this.helpers({
            parties: () => { return Parties.find(params.selector, params.options); },
            searchCriteria: this.searchCriteria
        });

        this.subscribe('parties', () => ['getParties', this.searchCriteria]);
    }
}

function searchDirective(): angular.IDirective {
    return {
        templateUrl: 'client/components/search/search.ng.html',
        controllerAs: 'search',
        scope: true,
        bindToController: true,
        replace: true,
        controller: SearchCtrl
    };
}

angular.module('app').directive('search', searchDirective);

This code assumes that .this.helpers() actually make sub-properties of the searchCriteria object reactive too (which isn't currently working in 1.3.0).

It would be ideal for me (and I'm sure for a lot of developers out there too) if in the example above just worked. It seems that for that example to work we should go with either number 1 or 2. I personally would prefer number 1 so that we only use one method to deal with meteor related code. Also, having code written like that will ease up migration to Angular2, which is very important for me.

If we went with number 3 or 4, code would not be as smooth anymore. There would be some code duplication. I'd have to declare the searchCriteria at the beginning (like I'm doing now), and I'd have to pretty much declare it again in the .helpers function also specifying which props are reactive.

EDIT: Not really related, but I also think that the queryConstructor principles mentioned above should be somewhere in the "best practices" section of angular meteor docs.

[tomups]

I agree with @DmitryEfimenko that the most intuitive way is to have helpers be deep by default. I think it is what most people expect and what will be needed in 90% of the cases. For the 10% of not using deep for performance, how about choosing it through the helper function? Have an argument that would set if the helper is deep or not. So solution 1 with an extra "deep" argument, defaulting to true.

//deep helper
$scope.helpers({
    foo: {...}
}, true);

//deep helper
$scope.helpers({
    foo: {...}
});

//shallow helper
$scope.helpers({
    foo: {...}
}, false);

[Urigo]

@DmitryEfimenko good points (and I love your use of Typescript) but I wonder if that's really more code to use getReactively. We will solve the nested problem in helpers anyway in 1.3.1 but I want to have one way of doing things. If you declare variables exactly like use would with Angular then just add reactivity when needed then it is more clean and easier to migrate to Angular 2.0. In your solution it's like you defined searchCriteria twice.. By the way, here is a suggestion on how using Reactive Vars in Angular 2.0 would look, I would love your opinion about that: https://docs.google.com/document/d/1tkhqvVJimXi5hKvKViNKaujkfbZKzPOQwj0uHQmzcBU/edit?usp=sharing

[DmitryEfimenko]

@Urigo Yea, I'm all for removing code duplication. So if you think the getReactively case would be cleaner I'll only support it. If the code would look something like the below, I'm all for it:

this.getReactively(this.searchCriteria, true); // second parameter for the deep watch
this.subscribe('parties', () => ['getParties', this.searchCriteria]).then(()=> {
    let params = queryConstructor('getParties', this.searchCriteria);
    this.parties = Parties.find(params.selector, params.options);
});

This actually does look cleaner! Not only that, it would probably solve the @shantanubhadoria's concerns regarding having one subscription depend on the results of another... (or would it?) I'm also unsure how to deal with such scenario and I'd love to know! If the code is going to be something different from above, please show us what it would be.

I wrote a bit on the Angular 2.0 doc. Thanks for the link!

[DmitryEfimenko]

I just realized, angular's $watch takes a string, so I think the getReactively approach that you are thinking about would look like:

this.getReactively('search.searchCriteria', true);

That's kind of unfortunate because of the magic string situation. Is there a way to make it work like in my previous comment?

[Urigo]

mmm maybe, that might be a good idea, care to have a look at the code? https://github.com/Urigo/angular-meteor/blob/master/packages/angular-meteor-data/modules/angular-meteor-reactive-scope.js#L22-L44

About your example, right now it will look like that:

this.subscribe('parties', () => ['getParties', this.getReactively('searchCriteria', true)]).then(()=> {
    let params = queryConstructor('getParties', this.searchCriteria);
    this.parties = Parties.find(params.selector, params.options);
});

So there is no need to define searchCriteria differently, it's just a regular Angular property. When you want to use that property as reactive, then you wrap it with getReactively. And when you just want to access it for it's value outside a reactive context. like you've done on the promise function, then you again use it as a regular Angular property.

I agree you will need to write a bit more when you use the variable in a reactive context (inside helpers), but I think it's more important to keep the Angular code as clean as possible and the Meteor code as clean as possible. It means that the interaction points are very clear and obvious to the developer and that existing documentation for both frameworks would still be valid here

[DmitryEfimenko]

I looked at the code and I see why it's problematic to make getReactively() accept the property itself rather than a property name... mostly because of this line... that's how Tracker.Dependency array is managed... by the property name. It's a shame we have to keep magic strings around.

Besides that, is it not possible to keep this.getReactively() code outside of the params of subscribe() function? Like the below:

this.getReactively('searchCriteria', true);
this.subscribe('parties', () => ['getParties', this.searchCriteria]).then(()=> {
    let params = queryConstructor('getParties', this.searchCriteria);
    this.parties = Parties.find(params.selector, params.options);
});

The code looks cleaner. As far as I can tell getReactively() just sets up Tracker.Dependency() and returns the original value (this.searchCriteria in my case).

[sean-stanley]

Just to be part of the discussion I would prefer the API to have helpers of type Object be deeply reactive by default without having to call $getReactively or similar. I admit I don't know if there is likely to be a huge performance hit but I want to watch reactively an object with only about 6 string properties.

I did actually write a workaround for this issue by the way. I use a string primitive helper in my reactive function and when I change my non-reactive properties of my object, I also simultaneously change my string helper. This forces Autorun to run again with my new object's values. I know it's not a pretty work around and quite specific to my use case but hopefully it will be of use to someone waiting for this issue to be officially solved.

$scope.helpers({
    changeMe: "",
    sort: {name:1},

    products() {
      let change = $scope.changeMe;
      let query = {}
      options = {
        sort: $scope.sort
      };
      for(let key in $scope.stateParams) {
        if ($scope.stateParams[key] != null) {
          query[key] = $scope.stateParams[key]
        }
      }
      $log.info($scope.stateParams)
      return Products.find(query, options)
    },
    categories() {
      return Categories.find()
    }

  })

Then in my view I use $scope.changeMe like this:

<md-menu-item ng-repeat="c in categories">
     <md-button ng-click="stateParams.category = c.name; changeMe = c.name ">
              {{c.name}}
      </md-button>
</md-menu-item>

[mattiLeBlanc]

Question: how the Reactivity going to work with a model change? For example, having a datepicker

md-datepicker(name="dateField" ng-model="vm.filter.fromDate" md-placeholder="Enter date" required md-min-date="minDate" md-max-date="

When the user selects a date, now I get a notification of that by using autorun:

   $scope.$meteorAutorun ->
      console.log($scope.getReactively( 'vm.filter.fromDate') )

Is this going to be 'bad practice'? A helper doesn't seem to make sense because a helper (in Blaze) is mainly there to print stuff in templates, right? So a model update doesn't fit in that category.

I have been reading about $reactive (http://www.angular-meteor.com/api/1.3.0/reactive) and (http://www.angular-meteor.com/tutorials/socially/angular1/3-way-data-binding but I am not entirely sure how to do the model changes. When I set up a helper that returns the fromDate it complains the VM model does not have a filter. It seems that on first run there is no model available.

  ( $scope, $reactive ) ->
    reactiveContext = $reactive( this ).attach( $scope )
    reactiveContext.filter = {}
    reactiveContext.helpers
      test: ->
        console.log("I run", reactiveContext)
        return reactiveContext.filter.fromDate

When I run this part of the controller, and change the value of the datepicker and THEN open the debug result and look at the filter object, it holds the correct values. However, a reRun of the $reactive is never triggered when I keep updating the date field. What Am I missing?

***** Edit** I think one of the issues I have is related to using Coffeescript, and controllerAs. When removing controllerAs I can preset models using the $scope.filter.fromDate approach. When using controllerAs the this.vm or scope.vm does not seem to exist. In my other projects I use es6 Babel style and there ControllerAs works perfectly and I can just access the this scope and set for example this.filter.fromDate.

[Urigo]

@DmitryEfimenko The thing is that with your approach, you are making searchCriteria always reactive. What happens if we use it in another autorun and there we don't want it to trigger a rerun? The goal to use getReactively is to define reactivity only when and where it is needed so it will be very obvious to the developer what is happening and why things are triggering reruns.

@sean-stanley we've fixed that issue and will release in 1.3.1 (hopefully this weekend) to make helpers for primitives be deep by default. but I've also added a warning that this will be deprecated in 1.4.0 in favor of the getReactively API which let's you choose between deep or not.

@mattiLeBlanc good catch - also fixed in 1.3.1 so you can use getReactively on this as well.

[DmitryEfimenko]

uh, I just really don't like how verbose it becomes... hard to read code. Even have to horizontally scroll your example (and we know that's never good) to see the whole thing.

But would I be correct in assuming that I could write something like this:

let reactiveSearchCriteria = this.getReactively('searchCriteria', true);
this.subscribe('parties', () => ['getParties', reactiveSearchCriteria]).then(()=> {
    let params = queryConstructor('getParties', this.searchCriteria);
    this.parties = Parties.find(params.selector, params.options);
});

[Urigo]

I've released the new 1.3.1 version. It has the API like I've said before - fixes the nested problem on helpers but recommend you to use getReactively. You gave really good points, but I think that the risks of making reactivity implicit when running a large scale app are greater then the benefit of writing a bit less code. I hope you will think the same when getting into a larger code base, but if not, I would love to keep the discussion and also maybe create a separate package with your helper implementation (you can copy our implementation). Thank you for all the great and valuable feedback!

[jacobdr]

+1 

Great community discussion. Looking forward to more of these as our community matures. 

— Sent from mobile

On Sat, Dec 19, 2015 at 9:42 PM, Uri Goldshtein notifications@github.com wrote:

Closed #955.

Reply to this email directly or view it on GitHub: https://github.com/Urigo/angular-meteor/issues/955#event-496714325

[mattiLeBlanc]

@Urigo I have used the $reactive dependency for a state with the subscribe function. (sorry for the coffeescript)

    $reactive( @ ).attach( $scope )

    @subscribe 'events'

    @helpers
      events: ->
        Events.find()
]

and I am using ControllerAs (vm) in my Ui-Route. When I want to do a ng-repeat of my events helper, I need to do

  div(ng-repeat="event in vm.context.events") {{ event.title}}

Why do I need to add context after my VM?

When I attach the helper directly to the $scope it also works without the VM (but I want to avoid using scope).

[hanselke]

@mattiLeBlanc mind showing me what the coffeescript @helper structures you needed up using?