canjs / can-view-scope

Scope management for view engines
https://canjs.com/doc/can-view-scope.html
MIT License
4 stars 0 forks source link

Fix broken scope walking #183

Closed BairDev closed 6 years ago

BairDev commented 6 years ago

I think we are stumbling upon a bug in this line.

The code in scope.read goes like

while (parent.isSpecial()) {
    parent = parent._parent;
}

Here parent will be undefined at some point and parent.isSpecial() with

Cannot read property 'isSpecial' of undefined (Chrome)

I'd go with while (parent && parent.isSpecial()) or something similar.

For some context: we are trying to use something like {{#if(myList.length)}} or {{#if(../myList.length)}} in a stache view. The workround is adding a ./ or a this. to the reference like {{#if(./myList.length)}}.

I know that we could and should use {{#each(myList}}...{{else}}...{{/each}}, but sometimes we either don't want to iterate or have a different reason for checking the length of a list before iterating over it, e.g. in a <ul> context.

justinbmeyer commented 6 years ago

Thanks. On my list for monday.

justinbmeyer commented 6 years ago

Just to make sure I understand, you have something like ../key where there is no parent context?

I'm not sure what we should do here. In some ways an error seems right.

An example might be:

var scope = new Scope({});

scope.read("../key",{ ... })

Should we warn or error?

Thoughts @phillipskevin / @chasenlehara / @matthewp ?

justinbmeyer commented 6 years ago

An error or warning might say:

MyAppView:1: Unable to find key '../key'. There is no context at ... Did you mean ./key?

matthewp commented 6 years ago

With leakScope a template might be used in cases where there is a parent context and in other cases where there is not, so I think a warning.

justinbmeyer commented 6 years ago

Fixed in: https://github.com/canjs/can-view-scope/releases/tag/v4.7.1