canjs / can-stache

Live binding handlebars templates
https://canjs.com/doc/can-stache.html
MIT License
10 stars 13 forks source link

Allow side-effectful call expressions without observation or caching #724

Open rjgotten opened 5 years ago

rjgotten commented 5 years ago

Proposal

Extend call expressions to pass in notWrappedInObservation flag from individual user-supplied helpers, allowing non-observed functions to return non-cached values.

Benefits

Allows users to author side-effectful helpers. One use-case is a helper that implements unique IDs as sequence-numbers; increasing with each call. E.g.

<div data-uuid="{{ uuid() }}"> ... </div>
<div data-uuid="{{ uuid() }}"> ... </div>

This currently doesn't work, as call expressions are always wrapped in observations which cache their result value; and the observations are in turn put into the expression evaluator cache. I.e. both uuid calls above will return the same result.

Strip away the observation; turning it into a plain function, and things do work.

Drawbacks

None that I am aware of. The code to add this isn't even that many lines to adapt.

Migration

This can be implemented strictly as a feature toggle that requires opt-in on individual helper functions. This means it wouldn't break anything.

Implementation details

Call.prototype.value has a small piece of code that would need to be adjusted:

https://github.com/canjs/can-stache/blob/942724f6cd15b8ea5ab07f7f186ee3b47162282f/expressions/call.js#L117-L123

This would become:

if (helperOptions && helperOptions.doNotWrapInObservation) {
  return computeFn();
} else if (func.doNotWrapInObservation) {
  // Checking this flag once up front, on func should be conceptually OK.
  // Either the flag is set and won't change - because non-observed - or 
  // it is not set, meaning we will create an observation around the function
  // and keep that alive - i.e. the flag won't ever be respected even if it does
  // end up being set.
  // Note that call expressions still require the actual function to be returned
  // as the evaluator, which is why this case is separate from the previous one.
  return computeFn;
}
  var computeValue = new SetterObservable(computeFn, computeFn);

  return computeValue;
}
justinbmeyer commented 5 years ago

This behavior is different for "magic" attributes and "magic" between-tags as shown here:

https://codepen.io/justinbmeyer/pen/wvvBGEo?editors=1111

This is due to the catching @rjgotten mentioned. This asymmetry isn't great.

phillipskevin commented 5 years ago

Yeah, this is confusing. To summarize:

justinbmeyer commented 5 years ago

@phillipskevin yeah, attributes cache the compute. This helped the performance of the "spinner" example as we wouldn't need to rebuild computes (and all the scope walking) for something like:

<div id="{{number}}" style="left: {{number}}px" alt-thing="{{number}}">

I don't remember why this wasn't also done for "magic between-tags". Maybe for a reason similar to this.

An alternate approach would be to only cache computes when they have dependencies.

// NOT CACHED
var compute = new Observation(()=> Math.random() ) 
compute.hasDependencies() //-> false

// CACHED
var compute = new Observation(()=> Math.random()*obs.value ) 
compute.hasDependencies() //-> true

This would allow people to opt out with ObservationRecorder.ignore() too:

var compute = new Observation( ObservationRecorder.ignore( ()=> {
  return Math.random()*obs.value
}) ) 
rjgotten commented 5 years ago

An alternate approach would be to only cache computes when they have dependencies.

That would actually still break if you have something side-effectful that also observes, right? 😞 Granted; that would also break in my own proposal and yes, you could avoid it with ObservationRecorder.ignore.

However, after having given this some more thought over the course of the day, maybe something altogether different would work to solve the general problem. I'll explain.


Currently there's a metadata.rendered which can be set to true to signal that the helper will return a callback function( el ) {} expecting the rendered DOM element to do some work on. Ofcourse; that's specific to contexts where an element exists.

So... what if you allow helpers in general to return a function for a late-evaluated effect? The rendered flag is really just a special case - particular to needing a DOM element reference - of the general purpose of late bound evaluation, isn't it?

What if a conceptual metadata.late = true could signal the view engine that whatever value is spit back out of a call expression evaluator consists of a function that needs to be run and its that function's output which should be used by the magic tag?

In such a case, all the parts that need to be observable can be inside the outer helper. And the parts that are side-effectful can be inside the returned function.

Quick draft for the uuid example and how that would work out:

import { stache } from "can";

const uuid = 0;

function uuid( prefix = "", options ) {
  if ( stache.isOptionsLike( prefix ) {
    [ prefix, options = [ "", prefix ];
  }

  options.metadata.late = true;
  return () => prefix + (++uuid);
}

uuid.requiresOptionsArgument = true;

export { uuid as default };

Here the observable (and bound/cached) part is in the outer function which is made observable and whose result is cached in the evaluator cache. This observable then returns a function which is uncached and unobserved and facilitates the side-effectful sequence number.

The general logic flow would still remain: pull the observable from the evaluator cache; call can.getValue and grab the bound/cached result. But, if options.metadata.late indicates late-evaluation and that result is a function, then call said function and return its value - instead of directly returning the observable's result.