Using observeAll with a filter

[BrentH-Alnico]

Hi Boris,

I couldn't find a sample using a filter with observeAll...its got me a bit confused ;-) https://www.jsviews.com/#observeAll@filter

Case #⁠1 root props are passed through name (unexpected) while objects/arrays address are not (expected).
Are root props filterable?...they pass through in all cases below.

$.observable(person).observeAll(changeHandler, function (path, object, parentObs) {
   return false; // Does not prevent root props from passing through
});

Case #⁠2 root.address passes through (expected) while root.phones[] or root.phones[0] does not (unexpected).

if (path == 'root.address' || path == "root.phones[]" ) {
   return true;
}
return false;

Case #⁠3 both root.address and root.phones[] are not passed through (expected) while root.address2 is passed through (expected).
Notice that path root.phones[] works here, but not in the above case.

if (path == 'root.address' || path == "root.phones[]" ) {
   return false;
}
return true;

Here is one of your examples slightly modified for testing: https://jsfiddle.net/alnico/t3fv5ngk/1/

My use case (case #⁠3) using a filter, is to listen to all 'business data' changes that would enable a 'Send' button, while excluding UI opened/closed state data, etc.
I have much more business data than other...thus I would like to use this as an exclusion filter.

[BorisMoore]

Thanks for finding this, Brent. I am looking into it.

[BorisMoore]

Hi Brent, what I am finding is that the observeAll filter feature is not fully, or correctly implemented. In the documentation it says:

Sometimes a scenario requires that $.observable(rootObject).observeAll(handler) calls the handler for a subset of the observable changes under the rootObject, rather than for every change.

In fact we are concerned with two different timeframes here.  First the timeframe of the observeAll() stepping through the hierarchy and attaching the handler to each object/array in the tree. Second, the timeframe of observable changes occurring anywhere in the tree, triggering calls to the handler. The documentation is not very clear as to which of those timeframes are concerned. 

The implementation of the observeAll filtering feature is actually concerned with the event binding timeframe. This filtering allows you to exclude chosen objects (and the underlying subtree) from the event binding. It does not in fact allow you to exclude individual properties of type string, number, etc. such as the name string property. 

Your scenario for filtering out non-business data seems to match the observable changes timeframe, and it can in fact be addressed directly, I think, without using the filter feature, along the lines of:

$.observable(person).observeAll(filterEvents);
function changeHandler(ev, eventArgs) {
  let message = "";...
  $(".messages").append("<div>" + message + "</div>");
}

function filterEvents(ev, eventArgs) {
  const fullPath = ev.data.observeAll.path() + (eventArgs.path
    ? "." + eventArgs.path  // Object: setProperty
    : "[]"                  // Array: insert/remove
  );
  
  if (!ignorePaths.includes(fullPath)){
    changeHandler(ev, eventArgs);
  }
}

I am leaning towards the option of removing the whole feature. The alternative is to provide the feature (after fixing a couple of bugs in the implementation), but it will not be needed for your specific scenario, and I think is more of a potential source of confusion,  rather than addressing a real need.

What do you think?

[BorisMoore]

Here is my current version, which keeps the filter feature, but only calls the filter callback for objects or arrays.

(The jsviews15 also calls the filter callback from leaf properties such as string-value street: "1st Ave", but returning true or false from the callback actually does nothing!)

jsviews16 Candidate G.js.txt

[BrentH-Alnico]

Hi Boris, thanks for the clarification on the filter feature; when I first read the doc...

...calls the handler for a subset of the observable changes under the rootObject

...initially I thought there may be some limits, but it didn't make sense to me what the use cases would be and therefore went down a rabbit hole thinking that it was implemented wrong or buggy ;-)

Anyway, yes doing direct filtering as you suggest should work fine; I haven't had a chance to dig into it yet.

Whether to keep the filter feature or remove it: since it was never implemented correctly/fully as you said, are you assuming that nobody is currently using it and therefore removing it wouldn't be an issue?

I would keep it only if there was a performance gain due to less binding/listening, but my guess it's probably negligible?

I actually have a case where I 'could' use it, where I want to exclude an array of objects that are two-way bound where the data is posted directly via modal popups. But, since I also want precise filtering across my entire app (using methods like you mentioned)...I think this would add to the complexity without much value.

If you do decide to keep it, I think the documentation needs to be more clear describing its intent and limitations (I would have not opened this issue had I understood its use ;-)...maybe something like this:

Sometimes a scenario requires that $.observable(rootObject).observeAll(handler) calls the handler for a subset of the observable changes under the rootObject. This selective behavior can include or exclude certain objects or arrays from being bound during initialization, which helps reduce the overhead of observing unnecessary parts of the object graph.
Note: This does not prevent listening to changes on individual (leaf-level) properties, including those directly on rootObject.

However, I do think having a filtering feature built-in like I initially imagined would be convenient...I would think this is a common need.

Furthermore...

The direction I am taking this filtering, is to create a log of dataChanges that includes the fullPath/prop, oldValue and newValue (for business data only).
If dataChanges.length...then enable the Send button.
If a new change comes with the oldValue...then disable the Send button, etc.

This approach creates incremental snapshots of data changes as opposed to using unmap() (view models only). This would give more nuanced control over undo/redo, revert, form/parts resets in appropriate scenarios and I think should perform well...I have yet to test out these ideas ;-)

[BrentH-Alnico]

Hi Boris,

...In the case that you are actually adding a filter feature using path/prop 👍 ...

I have a related feature request: the ability to pass this array of paths/props to unmap() for exclusion from the final output.

This would be useful and convenient for cutting down the amount of non-business data posted to the server.

For plain objects/arrays a person would have to write their own script.

I just wanted to put this out there to perhaps including in your next version...
Sorry if I'm being presumptuous ;-)

Best,
Brent

[BorisMoore]

Hi Brent, no not presumptuous! :-).
It's an interesting idea to consider being able to in some sense apply 'the same filtering' to observeAll() and to unmap(), and maybe also to map().

I am still working on fixing the currently incomplete/partly incorrect filter feature. It may be that for the next release (1.0.16) we need to just keep it as is, just in case some people are managing to use the current implementation effectively. (I don't think performance is a primary reason for or against...)

But I would consider changing it in a subsequent 1.1.00 update. Not sure yet whether that could already integrate with unmap() as you suggest, or whether we are really talking about v2.0.

[BrentH-Alnico]

Hi Boris,

If you are leaning towards building in the filter feature, some ideas...

Current signature $.observable(objectOrArray).observeAll(myHandler[, myFilter])

Propose signature $.observable(objectOrArray).observeAll(myHandler[, myTarget, myFilter])

If no targets then pass undefined

Even though we believe that the current signature myFilter has little value (unknown in the real world), perhaps you could keep it...as myTargets (relabeled)...and thus no breaking changes.

Also, $.observe(...) could technically utilize the same myFilter I believe, if the signature would work. But I'm not sure how useful it would be there.

Regardless, if you don't build in the filter, passing paths/props to unmap() and map() would still be beneficial of course. I never really thought about passing to map()...I actually think that might be useful.

Yeah that makes sense to separate this into its own version if indeed you are going to remove it.

[BorisMoore]

Hi Brent,

Thanks for your additional thoughts/scenarios/requests. I spent some time on trying to restructure the code so as to provide a stable and consistent implementation for this filtering concept. However it is not straightforward, because the same feature is used internally for https://www.jsviews.com/#tagoptions@datamap, and fixing it for your scenario risks breaking it for the {{for}} and {{props}}. So finally I am keeping the current implementation, but adding a deprecated note in the docs https://www.jsviews.com/#observeAll@filter:

(Note: This feature is currently deprecated. An improved version may be provided in a future update.)

[BrentH-Alnico]

If I understand correctly, the "...same feature is used internally" and must remain...
Are you then suggesting that when "myFilter" is deprecated that it "may" be possible to add the filtering concept...in the future?

Anyway, I will experiment building it out-right and then passing paths to a script to remove data from the the unmapped json...I will report findings.

Also, a few months ago I experimented with defining these "non-business props" in my viewModel(s)...
{ getter: "unmapExclude", defaultVal: ["selectedIndex","msg"] } where I then used js to remove the the array props and the getter prop itself from the unmapped data, it worked fine.

I then experimented where they could be excluded during the unmap() process (I used hard coded data which seemed to work)...with the idea of using it as unmap("unmapExclude",...)...

But, the observeAll: myFilter concept and ability to use the same paths with unmap() is ideal.

Thanks for investigating this filtering concept and all other recent fixes...will be nice to get them published I'm sure ;-)

[BorisMoore]

I have just published an update to the github repos: jsviews.com, jsviews and jsrender. So the www.jsviews.com site is using the v1.0.16 update, which can be downloaded from there: https://www.jsviews.com/#download. I have not yet published to npm or the CDN.

Yes, it's really nice to finally get all those recent updates/fixes published! I hope all is working correctly. As always, let me know if you see any issues.

And yes, the note deprecating the observeAll filtering feature is here, and it signifies simply that there may be issues with the current implementation, and that we hope to provide a future update. (And implicitly we are warning that that update may be a breaking change...)

So my hope is that we can provide a 'unified' implementation that provides a common code path for filtering on observeAll(), unmap(), and possibly also map() and the dataMap feature used in {{for}} and '{{props}}'. However that work will be something that goes beyond the scope of this minor release, which is why I reverted to keeping them separate...

So I suggest we keep this issue open, to track that 'future goal'. All the other issues will be closed (once npm publishing, etc are complete.) Meantime, any investigation and coding that you are working on concerning these filltering scenarios can potentially be valuable input for that 'hoped for' future version !!

Thanks for you help in calling out these various issues!

[BrentH-Alnico]

Hi Boris, you mentioned...

not yet published to npm or the CDN

And then the next day/two the notification of 1.0.16 went out...
1.0.16 is still not available on CDN, is there a reason?
...1.0.16 is working for me, no issues found.

Yes, the path to observeAll() filtering sounds good.

Thanks,
Brent

[BorisMoore]

Yes, correct, the 1.0.16 update has now been published to NPM. It is available (fortunately) on the jsDelivr CDN at:

jsdelivr.com/package/npm/jsrender
and
jsdelivr.com/package/npm/jsviews

Unfortunately the CDNJS CDN bot that is supposed to automatically pick up new versions from NPM has not been working, apparently for weeks:
cdnjs/cdnjs#14263 (comment)

Quite a pain! No information is being provided as to when it might be fixed....

I am in the process of updating the jsviews.com site to document the alternative jsDelivr CDN option....