ractivejs / ractive

Next-generation DOM manipulation
http://ractive.js.org
MIT License
5.94k stars 397 forks source link

Supported API to add computed dynamically #3120

Closed Jeff17Robbins closed 6 years ago

Jeff17Robbins commented 6 years ago

Description:

We are close to general release of a healthcare app that uses Ractive. We're looking forward to a Ractive 1.0 official release, and are hoping to remove the one undocumented feature we are using:

ractive.viewmodel.compute()

We need to add computeds that on-the-fly to an already existing Ractive instance. It would be great if there were a supported API to do this!

Versions affected:

1.0

Platforms affected:

Reproduction:

fskreuz commented 6 years ago

It's so undocumented, I wasn't even aware this exists. 😁

Jeff17Robbins commented 6 years ago

Yeah, @paulie4 and I were trying to remember how we learned about it. I think there is an old conversation about it from someone else asking for the same feature.

If anyone is curious, our use-case is that we want to sort some data that arrives from the server. We started out using a helper function to do the sort -- this works great in a template. But...not so great in JavaScript ractive.get(). Whereas a computed works great both in templates and in ractive.get().

evs-chris commented 6 years ago

You can actually get a handle to a template expression using a context pretty easily, and it's very nearly the same as a computed. Not that adding computeds dynamically isn't something we should support. It just wasn't possible until 0.8, I think.

I'll see what it would take to make an official API for 1.0. I don't think it's quite as simple as exposing the viewmodel compute, as that wouldn't handle transferring any existing deps to the new model.

Jeff17Robbins commented 6 years ago

I'm unfamiliar with getting a handle to a template expression. Happy to move to a different feature if it does what we want and is official!

As for "transferring any existing deps to the new model"...well...if I knew what that meant I bet I would feel smarter! :-)

On Thu, Oct 26, 2017 at 9:26 PM Chris Reeves notifications@github.com wrote:

You can actually get a handle to a template expression using a context pretty easily, and it's very nearly the same as a computed. Not that adding computeds dynamically isn't something we should support. It just wasn't possible until 0.8, I think.

I'll see what it would take to make an official API for 1.0. I don't think it's quite as simple as exposing the viewmodel compute, as that wouldn't handle transferring any existing deps to the new model.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ractivejs/ractive/issues/3120#issuecomment-339846291, or mute the thread https://github.com/notifications/unsubscribe-auth/ACAWh9pZV1WD3oSOLBt2RTmDW6kyO4bUks5swTFEgaJpZM4QIaPy .

evs-chris commented 6 years ago

For the context handle, if you have something like

{{#with some(computation)}}
<div id="mah-expr">...</div>
{{/with}}

you can get stuff from the expression by getting the divs context with var ctx = Ractive.getContext('#mah-expr') and using ctx.get('.exprProp'). This works because the context for the div is the expression model that's created in the background and owned by the template.

Jeff17Robbins commented 6 years ago

Made a fiddle to try to understand http://jsfiddle.net/vt76xm54/

HTML

<div id='target'>

</div>

JAVASCRIPT

var ractive = new Ractive({
  template:'{{sort(items)}}',
  target: '#target',
  data: {
    items: [ 2, 10, 200, 3, 1, 4],
    sort: function(arr) {
      var newArray = arr.slice();
      return newArray.sort(function(a, b) {return a - b;});
    }
  }
});

var ctx = Ractive.getContext('#target');
console.log(ctx.get());

I see my sorted array on the screen thanks to {{sort(items)}} And I see the results of my ctx.get() in the console.

But I don't understand how to ctx.get( <sorted items> ).

paulie4 commented 6 years ago

For Ractive.getContext() to work usefully, it needs to be given an element within a Ractive template context. The places where we use an array of data almost always translates to at least one DOM element per array item, but your example is just returning the array values as a single string, which makes Ractive.getContext() useless for getting each item. Here's the example using an {{#each}} and iterating over its children, http://jsfiddle.net/vt76xm54/1/:

var ractive = new Ractive({
  template:'{{#each sort(items)}}<span>{{.}}</span><br>{{/each}}',
  target: '#target',
  data: {
    items: [ 2, 10, 200, 3, 1, 4],
    sort: function(arr) {
      var newArray = arr.slice();
      return newArray.sort(function(a, b) {return a - b;});
    }
  }
});

Array.prototype.forEach.call(document.querySelectorAll('#target span'), function(elt) {
    var ctx = Ractive.getContext(elt);
    console.log(ctx.get());
});

We need to do some JavaScript work with the sorted array. It seems kinda roundabout to have to dig into the DOM and get each context to be able to get the data.

@evs-chris, I see that a Computation's prototype is Model, so I understand that a new Model will be created when ractive.viewmodel.compute() is called, but I also see that it has it's own array of dependencies that are calculated in Computation.prototype.getValue(), so I don't understand why a new Computation needs to "handle transferring any existing deps."

evs-chris commented 6 years ago

It is roundabout and definitely not ideal, but it is possible :smile:. Here's @Jeff17Robbins fiddle updated to grab the context without having to worry about iterations.

As far as compute goes, it isn't aware of any other models that happen to exist in the same keypath. If you have a template {{thingy}} that gets bound and later you ractive.compute('thingy', { get() { ... } }), then the {{thingy}} ref in the interpolator won't be bound to your new computation. Making support official would mean that at the end of the computation add process, the computation would tell any deps on the plain thingy model to start using it instead.

Since this would require a new instance method (thinking ractive.compute or ractive.addComputed), I was also thinking about whether or not this same API could be used to allow nested computations e.g. ractive.addComputed('some.list.*.computedProp', { ... }) a la #1583.

dagnelies commented 6 years ago

One thing I wonder is: why not directly sort the original array? Why go around it with computeds?

paulie4 commented 6 years ago

The original data is not an array; it's a keyed object. We keep our back-end and front-end synchronized by sending data diffs that require the data to be in an object format, so it has to then be converted into a sorted array for some front-end uses.

evs-chris commented 6 years ago

I have a branch for this, #1583, and a few others at https://github.com/ractivejs/ractive/tree/wild-compute if anyone wants to give it a poke. It only has basic testing in place because I haven't found the time to write the ton of tests that will be needed to cover the changes. It's also breaking, so it can't really land until 1.0.0. The PR text is also going to be quite large due to the number of changes and questions related to said changes.

Jeff17Robbins commented 6 years ago

Exciting! I'll try it out. The cleaner compute syntax looks great!

The explicit 'helpers' also makes a whole lot of sense.

On Tue, Nov 14, 2017, 4:04 PM Chris Reeves notifications@github.com wrote:

I have a branch for this, #1583 https://github.com/ractivejs/ractive/issues/1583, and a few others at https://github.com/ractivejs/ractive/tree/wild-compute if anyone wants to give it a poke. It only has basic testing in place because I haven't found the time to write the ton of tests that will be needed to cover the changes. It's also breaking, so it can't really land until 1.0.0. The PR text is also going to be quite large due to the number of changes and questions related to said changes.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ractivejs/ractive/issues/3120#issuecomment-344397882, or mute the thread https://github.com/notifications/unsubscribe-auth/ACAWh8umEqaLWeP2FkiyPBCR4YN90idGks5s2gBYgaJpZM4QIaPy .

Jeff17Robbins commented 6 years ago

A question about intent:

Is the dynamic compute() function meant to setup a given computed keypath once? Or is it intended to allow me to redefine a computed?

For example, right now I can call

        `r.compute('b', 'a * 2');`

And 'b' does what I expect. But if I then call

        `r.compute('b', 'a * 3');`

it doesn't seem to redefine 'b'.

On Tue, Nov 14, 2017 at 4:04 PM Chris Reeves notifications@github.com wrote:

I have a branch for this, #1583 https://github.com/ractivejs/ractive/issues/1583, and a few others at https://github.com/ractivejs/ractive/tree/wild-compute if anyone wants to give it a poke. It only has basic testing in place because I haven't found the time to write the ton of tests that will be needed to cover the changes. It's also breaking, so it can't really land until 1.0.0. The PR text is also going to be quite large due to the number of changes and questions related to said changes.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ractivejs/ractive/issues/3120#issuecomment-344397882, or mute the thread https://github.com/notifications/unsubscribe-auth/ACAWh8umEqaLWeP2FkiyPBCR4YN90idGks5s2gBYgaJpZM4QIaPy .

evs-chris commented 6 years ago

It does now :)

Jeff17Robbins commented 6 years ago

cool!

On Fri, Nov 17, 2017 at 3:54 PM Chris Reeves notifications@github.com wrote:

It does now :)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ractivejs/ractive/issues/3120#issuecomment-345362560, or mute the thread https://github.com/notifications/unsubscribe-auth/ACAWh-bJalfqe4uyLNJH6E3VyWwSOyY8ks5s3fJ0gaJpZM4QIaPy .

Jeff17Robbins commented 6 years ago

Dynamic compute redefinition works. However, I'm seeing one possible anomaly:

        var r = new Ractive({
            target:   '#target',
            template: '#template',
            data: {
                a: 1
            }
        });

        r.observe('b', function(n, o, k) {
            console.log(r.get());
            console.log(r.get('b', { virtual: true }), n, o, k);
        });

        function computed1() {
            r.compute('b', '`the value of a*2 (where a=${a}) is ${a * 2}`');
            r.add('a', 1);
        }

        function computed2() {
            r.compute('b', '`the value of a*3 (where a=${a}) is ${a * 3}`');
            r.add('a', 1);
        }

It appears as though b isn't get-able via r.get(). Meaning, in the .observe() callback,

console.log(r.get('b', { virtual: true }), n, o, k);

I was expecting r.get('b', { virtual: true }) to get the current value of b, but it logs undefined instead.

But r.get() shows b in its results.

//             console.log(r.get());
20:49:28.135 test.html:34 {a: 3, b: "the value of a*2 (where a=3) is 6"}

//             console.log(r.get('b', { virtual: true }), n, o, k);
20:49:28.136 test.html:35 undefined "the value of a*2 (where a=3) is 6" "the value of a*2 (where a=2) is 4" "b"
Jeff17Robbins commented 6 years ago

@paulie4 pointed out that my example is confusing.

An attempt at a simpler example:

        var r = new Ractive({
            target:   '#target',
            template: '#template',
            data: {
                a: 17
            },
            computed: {
                c: '`the value of a*a (where a=${a}) is ${a * a}`'
            }
        });

        r.compute('b', '`the value of a*2 (where a=${a}) is ${a * 2}`');

        console.log('r.get()', r.get());

        console.log('r.get("a")', r.get("a"));

        console.log('r.get("b")', r.get("b"));

        console.log('r.get("c")', r.get("c"));

And its console log output:


07:32:50.275 test2.html:39 r.get() {a: 17, b: "the value of a*2 (where a=17) is 34", c: "the value of a*a (where a=17) is 289"}
07:32:50.276 test2.html:41 r.get("a") 17
07:32:50.276 test2.html:43 r.get("b") undefined
07:32:50.277 test2.html:45 r.get("c") the value of a*a (where a=17) is 289

@evs-chris, notice how r.get("b") returns undefined, despite the dynamic computed showing up in the template and returned by r.get().

evs-chris commented 6 years ago

@Jeff17Robbins there was a bug that was transferring the deps to the new computation, so that the template would update correctly, but not setting it up such that the computation would take precedent in future calls. That should be fixed now.

Jeff17Robbins commented 6 years ago

Looks good:


14:57:17.552 test2.html:39 r.get() {a: 17, b: "the value of a*2 (where a=17) is 34", c: "the value of a*a (where a=17) is 289"}
14:57:17.552 test2.html:41 r.get("a") 17
14:57:17.553 test2.html:43 r.get("b") the value of a*2 (where a=17) is 34
14:57:17.553 test2.html:45 r.get("c") the value of a*a (where a=17) is 289

Thanks!

PaulMaly commented 6 years ago

Since this would require a new instance method (thinking ractive.compute or ractive.addComputed), I was also thinking about whether or not this same API could be used to allow nested computations e.g. ractive.addComputed('some.list.*.computedProp', { ... })

In my solution, I use a single method to safely add/update Ractive's instance props. Examples:

// using with constructor

// single prop assignment
Ractive.assign('components', {
      list: require('./components/List')
}); 
// equals Ractive.components.list = require('./components/List')

// multi props assignment
Ractive.assign({
       defaults: {
             enhance: true,
             lazy: true
       },
       adaptors: {
              promise: require('./adaptors/promise')
       }
});  

/* equals
Ractive.defaults.enhance = true; 
Ractive.defaults.lazy = true; 
Ractive.adaptors.promise = require('./adaptors/promise') 
*/

// using with instance
const ractive = new Ractive();

// single prop assignment
ractive.assign('newMethod', () => {});
ractive.method();
// safe way to make this ractive.method = () => {};

ractive.assign({
      decorators: {
           modal: require('./decorators/modal')
      },
      components: {
          list: require('./components/list')
      }
});

So, we could use this api to assign anything to Ractive or Ractive's instance in a safe manner.

ractive.assign({
      computed: {
            fullName: () => {}
      }
});

Probably, it's most of all similar on Vue's mixins api: https://vuejs.org/v2/guide/mixins.html

I use this method also to inject mixins into options before instance creation:

const options = {
   el: '#app',
   template: require('./templates/parsed/app')
};

options.decorators = {
     modal: require('./decorators/modal')
};

options.components = {
     list: require('./components/list')
};

options.data = {
      post: []
};

Ractive.assign(options, require('./mixins/user'));

// this call doesn't assign any props to Ractive constructor directly, 
// just mixin user to options. Works most like Object.assign()
/* user mixin simple example:
module.exports = {
      data: {
           firstName: '',
           lastName: ''
      },
      computed: {
          fullName: () => {}
      },

};
*/

const ractive = new Ractive(options);
// final options = {
   el: '#app',
   template: require('./templates/parsed/app'),
   decorators: {
     modal: require('./decorators/modal')
   },
   components: {
     list: require('./components/list')
   },
   data: {
      posts: [],
      firstName: '',
      lastName: ''      
   },
   computed: {
          fullName: () => {}
    }
};

Hope, that my proposal is clear. I believe it's very simple, but powerful solution. I would be very glad if it once became a part of the Ractive.

evs-chris commented 6 years ago

3141 is now cherry-picked onto edge, to be released in 0.10 in the not-too-distant future. Feedback welcome!