BorisMoore / jsviews

Interactive data-driven views, MVVM and MVP, built on top of JsRender templates
http://www.jsviews.com/#jsviews
MIT License
856 stars 130 forks source link

Observable change error, when loading jquery.observable.js before jsrender.js, and using contextual parameters and sorted {{for}} #446

Closed johan-ohrn closed 4 years ago

johan-ohrn commented 4 years ago

Sorry about the poor title. Anyway I found this issue.

I have this for-tag:

{^{for ~blah="apa" ~tag.tagCtx.filteredCollection.filtered start=~tag.tagCtx.pagingStartIdx end=~tag.tagCtx.pagingEndIdx tmpl=~tag.rowTemplate /}}

I do something to trigger pagingEndIdx to change and this ends up calling this code in jquery.observable.js:

            observe: function(deps, linkCtx) { // Listen to observable changes of mapProps, and call map.update when change happens
                var map = this,
                    options = map.options;
                if (map.obmp) {
                    // There is a previous handler observing the mapProps
                    $unobserve(map.obmp);
                }
                map.obmp = function() {
                    // Observe changes in the mapProps ("filter", "sort", "reverse", "start", "end")
                    var newTagCtx = linkCtx.fn(linkCtx.data, linkCtx.view, $sub)[options.index]; // Updated tagCtx props and args
                    $.extend(options.props, newTagCtx.props); // Update props to new values
                    options.args = newTagCtx.args; // Update args to new values
                    map.update(); // Update the map target array, based on new mapProp values
                };
                $observable._apply(1, linkCtx.data, dependsPaths(deps, linkCtx.tag, map.obmp), map.obmp, linkCtx._ctxCb);
            },

The line var newTagCtx = linkCtx.fn(linkCtx.data, linkCtx.view, $sub)[options.index]; // Updated tagCtx props and args is the culprit.

$sub does not contain everything the compiled view function expect. If I instead change the above line to this: var newTagCtx = linkCtx.fn(linkCtx.data, linkCtx.view, $.views.sub)[options.index]; // Updated tagCtx props and args then it work.

The compiled view function look like this:

(function anonymous(data,view,j,u
) {
 // #Tags.DataGrid/if 1 for
return [
{view:view,content:false,tmpl:false,
    params:{args:['~tag.tagCtx.filteredCollection.filtered'],
    props:{'start':'~tag.tagCtx.pagingStartIdx','end':'~tag.tagCtx.pagingEndIdx','tmpl':'~tag.rowTemplate'},
    ctx:{'blah':'\"apa\"'}},
    args:[view.ctxPrm("tag").tagCtx.filteredCollection.filtered],
    props:{'start':view.ctxPrm("tag").tagCtx.pagingStartIdx,'end':view.ctxPrm("tag").tagCtx.pagingEndIdx,'tmpl':view.ctxPrm("tag").rowTemplate},
    ctx:{'blah':j._cp("apa","\"apa\"",view)}}]
})

Notice how it wants to call j._cp(...). That function is not present on $sub but it is on $.views.sub

I hope you can spot the error and a fix for it. If not I can try to reproduce the error with simpler code.

BorisMoore commented 4 years ago

Thanks, Johann. Another good find. That is a bug which arises in the case of having jquery.observable.js and jsrender.js and jquery,views.js as separate files, with jquery.observable.js loaded before jsrender.js. I have repro.

I believe that if you load jsrender.js first or use jsviews.js (single file) the issue will not occur.

I'll work to get a fix, then let you know here...

johan-ohrn commented 4 years ago

Great! Once I have this fixed I can push the new version into production.

BorisMoore commented 4 years ago

Hi Johan, I think this should fix the issue:

jsviews2.zip

Let me know if it works for you.

(It also includes a fix to jsrender-node.js, and your issue https://github.com/BorisMoore/jsviews/issues/447#issuecomment-629946649)

johan-ohrn commented 4 years ago

An other related potential? issue is the following. In jquery.views.js on line 3184:

contextOb = contextOb && $sub.tmplFn(delimOpenChar1 + ":" + contextOb[1] + delimCloseChar0, view.tmpl, true)(linkCtx.data, view);

The compiled function returned by tmplFn() have both j and u input parameters but the call only passes in data and the view. There are a number of places where a similar call is made without passing in the extra parameters. I can't actually remember if I had an issue with it or not. Just letting you know for your consideration.

BorisMoore commented 4 years ago

You are right!! Thanks...

It is difficult to make a bug scenario that encounters that as an issue, but I managed to do so with this:

{^{on ~util[0].swap ...}}

which I will add to the unit tests.

So I have fixed it in the update below, to pass in $sub:

contextOb = contextOb && $sub.tmplFn(delimOpenChar1 + ":" + contextOb[1] + delimCloseChar0, view.tmpl, true)(linkCtx.data, view, $sub);

The u parameter is intended to be undefined, but in this update I have eliminated the ,u from the generated functions. I previously intended to use u for undefined in generated code, but I don't seem to be leveraging that any more, so I think it is OK to remove it.

Here is the update:

jsviews3.zip

BorisMoore commented 4 years ago

This has been fixed in new release v1.0.7