Skip to content

Commit

Permalink
Merge pull request #1460 from evs-chris/gh-1457
Browse files Browse the repository at this point in the history
Make sure shuffling doesn't cause index changes to propagate to levels referencing a deeper index
  • Loading branch information
evs-chris committed Nov 11, 2014
2 parents 2cefd74 + a494c6e commit 5a87c6f
Show file tree
Hide file tree
Showing 6 changed files with 82 additions and 22 deletions.
4 changes: 3 additions & 1 deletion src/virtualdom/Fragment/prototype/rebind.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ import assignNewKeypath from 'virtualdom/items/shared/utils/assignNewKeypath';

export default function Fragment$rebind ( indexRef, newIndex, oldKeypath, newKeypath ) {

this.index = newIndex;
if ( newIndex !== undefined ) {
this.index = newIndex;
}

// assign new context keypath if needed
assignNewKeypath( this, 'context', oldKeypath, newKeypath );
Expand Down
3 changes: 2 additions & 1 deletion src/virtualdom/items/Section/_Section.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import findComponent from 'virtualdom/items/Section/prototype/findComponent';
import findNextNode from 'virtualdom/items/Section/prototype/findNextNode';
import firstNode from 'virtualdom/items/Section/prototype/firstNode';
import shuffle from 'virtualdom/items/Section/prototype/shuffle';
import rebind from 'virtualdom/items/Section/prototype/rebind';
import render from 'virtualdom/items/Section/prototype/render';
import setValue from 'virtualdom/items/Section/prototype/setValue';
import toString from 'virtualdom/items/Section/prototype/toString';
Expand Down Expand Up @@ -48,7 +49,7 @@ Section.prototype = {
firstNode: firstNode,
getValue: Mustache.getValue,
shuffle: shuffle,
rebind: Mustache.rebind,
rebind: rebind,
render: render,
resolve: Mustache.resolve,
setValue: setValue,
Expand Down
14 changes: 14 additions & 0 deletions src/virtualdom/items/Section/prototype/rebind.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import Mustache from 'virtualdom/items/shared/Mustache/_Mustache';
import types from 'config/types';

export default function( indexRef, newIndex, oldKeypath, newKeypath ) {
var ref, idx;

if ( indexRef !== undefined || this.currentSubtype !== types.SECTION_EACH ) {
ref = indexRef;
idx = newIndex;
}

// If the new index belonged to us, we'd be shuffling instead
Mustache.rebind.call( this, ref, idx, oldKeypath, newKeypath );
}
8 changes: 7 additions & 1 deletion src/virtualdom/items/Section/prototype/setValue.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ function reevaluateSection ( section, value ) {
// TODO can this be optimised? i.e. pick an reevaluateSection function during init
// and avoid doing this each time?
if ( section.subtype ) {
section.currentSubtype = section.subtype;

switch ( section.subtype ) {
case types.SECTION_IF:
return reevaluateConditionalSection( section, value, false, fragmentOptions );
Expand Down Expand Up @@ -105,21 +107,25 @@ function reevaluateSection ( section, value ) {

// Ordered list section
if ( section.ordered ) {
section.currentSubtype = types.SECTION_EACH;
return reevaluateListSection( section, value, fragmentOptions );
}

// Unordered list, or context
if ( isObject( value ) || typeof value === 'function' ) {
// Index reference indicates section should be treated as a list
if ( section.template.i ) {
section.currentSubtype = types.SECTION_EACH;
return reevaluateListObjectSection( section, value, fragmentOptions );
}

// Otherwise, object provides context for contents
section.currentSubtype = types.SECTION_WITH;
return reevaluateContextSection( section, fragmentOptions );
}

// Conditional section
section.currentSubtype = types.SECTION_IF;
return reevaluateConditionalSection( section, value, false, fragmentOptions );
}

Expand Down Expand Up @@ -257,7 +263,7 @@ function reevaluateConditionalSection ( section, value, inverted, fragmentOption
if ( doRender ) {
if ( !section.length ) {
// no change to context stack
fragmentOptions.index = 0;
fragmentOptions.index = undefined;

fragment = new Fragment( fragmentOptions );
section.fragmentsToRender.push( section.fragments[0] = fragment );
Expand Down
34 changes: 15 additions & 19 deletions src/virtualdom/items/shared/Resolvers/SpecialResolver.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,34 +6,30 @@ var SpecialResolver = function ( owner, ref, callback ) {
this.rebind();
};

var props = {
'@keypath': 'context',
'@index': 'index',
'@key': 'index'
};

SpecialResolver.prototype = {
rebind: function () {
var ref = this.ref, fragment = this.parentFragment;
var ref = this.ref, fragment = this.parentFragment, prop = props[ref];

if ( ref === '@keypath' ) {
while ( fragment ) {
if ( !!fragment.context ) {
return this.callback( '@' + fragment.context );
}

fragment = fragment.parent;
}
if ( !prop ) {
throw new Error( 'Unknown special reference "' + ref + '" - valid references are @index, @key and @keypath' );
}

if ( ref === '@index' || ref === '@key' ) {
while ( fragment ) {
if ( fragment.index !== undefined ) {
return this.callback( '@' + fragment.index );
}

fragment = fragment.parent;
while ( fragment ) {
if ( fragment[prop] !== undefined ) {
return this.callback( '@' + fragment[prop] );
}
}

throw new Error( 'Unknown special reference "' + ref + '" - valid references are @index, @key and @keypath' );
fragment = fragment.parent;
}
},

unbind: function () {} // noop
};

export default SpecialResolver;
export default SpecialResolver;
41 changes: 41 additions & 0 deletions test/modules/rebind.js
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,47 @@ define([
t.htmlEqual( fixture.innerHTML, '<p>0:a</p><p>1:b</p><p>2:c</p><p>3:d</p>' );
});

test( 'index rebinds do not go past new index providers (#1457)', function ( t ) {
var ractive = new Ractive({
el: fixture,
template: '{{#each foo}}{{@index}}{{#each .bar}}{{@index}}{{/each}}{{/each}}',
data: {
foo: [
{ bar: [ 1, 2 ] },
{ bar: [ 1 ] },
{ bar: [ 1, 2, 3, 4 ] }
]
}
});

t.htmlEqual( fixture.innerHTML, '0011020123' );

ractive.splice( 'foo', 1, 1 );

t.htmlEqual( fixture.innerHTML, '00110123' );
});

test( 'index rebinds get passed through conditional sections correctly', t => {
var ractive = new Ractive({
el: fixture,
template: '{{#each foo}}{{@index}}{{#.bar}}{{@index}}{{/}}{{/each}}',
data: {
foo: [
{ bar: true },
{ bar: true },
{ bar: false },
{ bar: true }
]
}
});

t.htmlEqual( fixture.innerHTML, '0011233' );

ractive.splice( 'foo', 1, 1 );

t.htmlEqual( fixture.innerHTML, '00122' );
});

};

});

0 comments on commit 5a87c6f

Please sign in to comment.