Using onError with deep linking - maintaining data binding

[Paul-Martin]

Hi Boris,

I'm wondering if linking with the {^{if}} tag has an unexpected semantic. Please see this jsfiddle: http://jsfiddle.net/PaulMartin/8dys5j16/

In the fiddle, I was expecting the {^{if}} block to reexecute if any of the path variables b,c changed.

Investigating the code, it seems to me that the issue lies in the if tag onUpdate handler. Even though the b object is changing, the handler is returning false and preventing update because it is only checking for truthiness and not checking whether the values have actually changed. I can see how this could be intended behavior, as ultimately an if statement is checking for truthiness. But in the context of data-linking I was expecting that changing b in {^{if a.b^c}} would cause the if block to rerender as the content of that block is dependent on b, even if the truthiness hasn't changed.

Once I understood why this was not rerendering, I attempted to change the implementation to use a {^{for}} tag, but that introduced new issues related to onerror handlers. You can see that jsfiddle here: http://jsfiddle.net/PaulMartin/c6qqnvxq/

Here I've attempt to use onerror with both top-level linking and a regular {^{for}} tag. The top-level case does not seem to generate the necessary try / catch handler, and consequently throws an exception when one of the path variables is null -- in this case 'b':

(function(data,view,j,u
/**/) {
 // data-link="{for a.b^c tmpl=\'#child\' onerror=\'\'}" 0 for
return [
{view:view,tmpl:0,
    params:{args:['a.b^c'],
    props:{tmpl:'\'#child\''}},
    args:[data.a.b.c],
    props:{tmpl:"#child"}}];
})

The {^{for}} tag based example handles the exception, but it would appear that it never added the necessary property change event for 'b' to the a object, and hence when 'b' is subsequently observably updated, the view is not refreshed.

The workaround I've settled on for now is to wrap the {^{for}} tag in an {^{if}} tag that checks for b:

{^{if a.b}}
    {^{for a.b^c}}
    ... etc ...
    {{/for}}
{{/if}}

But this seems like it could get messy as the path depth increases

// The longer the path, the more checks
{^{if a.b && a.b.c && a.b.c.d}}
... etc ...
{{/if}}

Given this, I'm not sure if: 1) The behavior of {^{if}} is the desired semantic - at least it wasn't obvious to me. 2) If {^{for}} with an onerror handler not binding to the non-null part of the path in the case of a deep link is correct - perhaps it should bind to the non-null parts of the path, given that it seems a primary use case for onerror is with path that might have null values 3) If the top-level for with an onerror handler throwing when a portion of deep link path is correct. I don't think so, but maybe top level data links don't support onerror. 4) What the correct way might be to handle this scenario.

Thanks for taking the time to review this issue.

ps. I mentioned on twitter a few weeks ago how much I liked using .depends as a function. I still intend to send you some samples of how I use it, when I get a free moment. Been very very busy!

[BorisMoore]

Hi Paul,

Yes this is very useful. I appreciate the time you took to investigate and explain. I'll look more closely and get back to you.

[BorisMoore]

Hi Paul,

The latest update (commit 59) should address these issues. It was not easy, but I have added comprehensive support for data-linking with deep paths and using "onerror" so that it now supports dynamically moving between valid and invalid paths (missing objects deeper in path) while maintaining full data-binding to the whole path.

Here is a jsfiddle which shows the new behavior: http://jsfiddle.net/BorisMoore/6L3fkezb/.

I changed the title of this issue to correspond to the new support.

For the '{^{if}}' semantics the sample shows the behavior:

{^{if a.b^c onError=~error + 'D '}}
    If block (re-renders only if error or boolean value changes).
    Static: <i>{{:a.b.c.val}}</i> Linked: <i>{^{:a.b^c.val}}</i>
{{/if}}

So yes, it is by design that if you change the a.b.c to a new value but both the old and new values are valid complete paths and the leaf value is truey (in this case, an object) then the block will not rerender. The data context with the {{if}} block has not changed (since unlike {{for}} there is no 'navigation semantics' associated with the {{if}} - so for perf and preservation of state, we don't re-render. OTOH if something within the block also depends on the a.b.c path, then you can data-link that content, as in {^{:a.b^c.val}}.

Hopefully the new implementation provides the support you need for your scenarios. Let me know if it works for you!

Thanks again for bringing up this scenario.

[BorisMoore]

@Paul-Martin: I didn't hear whether this fix worked for you, but I'll close it now, assuming the fix is good (in general, and for your specific requirements...)/

[Paul-Martin]

Hi Boris,

Sorry I didn't respond immediately. The fix did work in some cases and did not work in other cases. I've been trying (unsuccessfully) to understand why one works and the other does not. As best I can tell the only difference between the 2 cases is that the working case has a path depth of 3, while the non-working case has path depth of 4.

You can see an example of the working case using this fiddle: http://jsfiddle.net/PaulMartin/ssr57h6v/

Notice the depth 3 path is

{^{for a^b().c() onerror='' }}

You can see an example of the non working case using this fiddle: http://jsfiddle.net/PaulMartin/oez26Lqn/

Notice the depth 4 path is

{^{for root.a()^b().c() onerror=''}}

Both examples are using computed observables. The primary difference that I can discern is the failing example has an additional computed observable in the path. The code appears to be recursive, so I have a hard time believing that depth of path is causing this case to fail. But after much debugging I haven't been able to track it down. Surely I'm overlooking some other difference.

Thank you so much for quickly fixing this problem. I know it is non-trivial. It does work for some of my cases, but not for this case.

[BorisMoore]

Thanks Paul, I'll look into it...

[BorisMoore]

@Paul-Martin: Hi Paul - I have an updated version that may be good, but as part of the testing, would you be able to try it in your environment? If so, could you email me at borismoore@gmail.com and I'll send you back the updated jsviews as an attachment...? Thanks!

[Paul-Martin]

HI Boris,

I'd be happy to test it in our environment.
Thanks for looking at this so quickly!

-Paul

On Oct 9, 2014, at 2:39 PM, Boris Moore notifications@github.com wrote:

Hi Paul - I have an updated version that may be good, but as part of the testing, would you be able to try it in your environment? If so, could you email me at borismoore@gmail.com and I'll send you back the updated jsviews as an attachment...? Thanks!

— Reply to this email directly or view it on GitHub.

[BorisMoore]

@Paul-Martin - OK - thanks for the testing Paul. Update 60 is now committed - with this fix, plus some additional fixes (including childTags() API). We can close this once you have confirmed it is working correctly for you.

[Paul-Martin]

Unfortunately commit 60 is not working for some of the cases that the prerelease file you gave me to test worked. I have not had a chance to look into why it's failing. I should have time to do so tomorrow.

On Oct 15, 2014, at 3:01 PM, Boris Moore notifications@github.com wrote:

@Paul-Martin - OK - thanks for the testing Paul. Update 60 is now committed - with this fix, plus some additional fixes (including childTags() API). We can close this once you have confirmed it is working correctly for you.

— Reply to this email directly or view it on GitHub.

[Paul-Martin]

Please disregard that last message. Commit 60 DOES resolve all issues.

On Oct 15, 2014, at 3:01 PM, Boris Moore notifications@github.com wrote:

@Paul-Martin - OK - thanks for the testing Paul. Update 60 is now committed - with this fix, plus some additional fixes (including childTags() API). We can close this once you have confirmed it is working correctly for you.

— Reply to this email directly or view it on GitHub.

[BorisMoore]

Ah good - had me worried :). Are we OK to resolve, or would you like to give it a bit more time to be sure?

[Paul-Martin]

We can resolve. If something else comes up we can reopen or open new as appropriate. Thanks so much for your fantastic work!

On Oct 15, 2014, at 5:41 PM, Boris Moore notifications@github.com wrote:

Ah good - had me worried :). Are we OK to resolve, or would you like to give it a bit more time to be sure?

— Reply to this email directly or view it on GitHub.

[BorisMoore]

OK - great. Resolving.