Skip to content

Commit

Permalink
Merge pull request #307 from sveltejs/gh-222-b
Browse files Browse the repository at this point in the history
Deconflict each-blocks contexts with reserved words
  • Loading branch information
Rich-Harris authored Feb 28, 2017
2 parents e486ed0 + 2165f08 commit c3ebf25
Show file tree
Hide file tree
Showing 8 changed files with 43 additions and 10 deletions.
8 changes: 7 additions & 1 deletion src/generators/Generator.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,13 @@ export default class Generator {
// noop
}

else if ( contexts[ name ] ) {
else if ( name in contexts ) {
const context = contexts[ name ];
if ( context !== name ) {
// this is true for 'reserved' names like `root` and `component`
code.overwrite( node.start, node.start + name.length, context, true );
}

dependencies.push( ...contextDependencies[ name ] );
if ( !~usedContexts.indexOf( name ) ) usedContexts.push( name );
}
Expand Down
8 changes: 4 additions & 4 deletions src/generators/dom/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class DomGenerator extends Generator {
properties.addBlock( `update: ${this.helper( 'noop' )},` );
} else {
properties.addBlock( deindent`
update: function ( changed, ${fragment.params} ) {
update: function ( changed, ${fragment.params.join( ', ' )} ) {
var __tmp;
${fragment.builders.update}
Expand All @@ -90,7 +90,7 @@ class DomGenerator extends Generator {
}

this.renderers.push( deindent`
function ${fragment.name} ( ${fragment.params}, component${fragment.key ? `, key` : ''} ) {
function ${fragment.name} ( ${fragment.params.join( ', ' )}, component${fragment.key ? `, key` : ''} ) {
${fragment.builders.init}
return {
Expand Down Expand Up @@ -180,7 +180,7 @@ export default function dom ( parsed, source, options, names ) {
contexts: {},
indexes: {},

params: 'root',
params: [ 'root' ],
indexNames: {},
listNames: {},

Expand Down Expand Up @@ -364,7 +364,7 @@ export default function dom ( parsed, source, options, names ) {
}

const names = [ 'get', 'fire', 'observe', 'on', 'set', '_flush', 'dispatchObservers' ].concat( Object.keys( generator.uses ) )
.map( name => name in generator.aliases ? `${name} as ${generator.aliases[ name ]}` : name );
.map( name => name in generator.aliases && name !== generator.aliases[ name ] ? `${name} as ${generator.aliases[ name ]}` : name );

builders.main.addLineAtStart(
`import { ${names.join( ', ' )} } from ${JSON.stringify( sharedPath )}`
Expand Down
2 changes: 1 addition & 1 deletion src/generators/dom/visitors/Component.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export default {
// Component has children, put them in a separate {{yield}} block
if ( hasChildren ) {
const yieldName = generator.getUniqueName( `render${name}YieldFragment` );
const { params } = generator.current;
const params = generator.current.params.join( ', ' );

generator.generateBlock( node, yieldName );

Expand Down
17 changes: 15 additions & 2 deletions src/generators/dom/visitors/EachBlock.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@ import CodeBuilder from '../../../utils/CodeBuilder.js';
import deindent from '../../../utils/deindent.js';
import getBuilders from '../utils/getBuilders.js';

const reserved = {
component: true,
root: true
};

export default {
enter ( generator, node ) {
const name = generator.getUniqueName( `eachBlock` );
Expand Down Expand Up @@ -172,16 +177,24 @@ export default {
const listNames = Object.assign( {}, generator.current.listNames );
listNames[ node.context ] = listName;

// ensure that contexts like `root` or `component` don't blow up the whole show
let context = node.context;
let c = 1;

while ( context in reserved || ~generator.current.params.indexOf( context ) ) {
context = `${node.context}$${c++}`;
}

const contexts = Object.assign( {}, generator.current.contexts );
contexts[ node.context ] = true;
contexts[ node.context ] = context;

const indexes = Object.assign( {}, generator.current.indexes );
if ( node.index ) indexes[ indexName ] = node.context;

const contextDependencies = Object.assign( {}, generator.current.contextDependencies );
contextDependencies[ node.context ] = dependencies;

const blockParams = generator.current.params + `, ${listName}, ${node.context}, ${indexName}`;
const blockParams = generator.current.params.concat( listName, context, indexName );

generator.push({
name: renderer,
Expand Down
2 changes: 1 addition & 1 deletion src/generators/dom/visitors/IfBlock.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ function getConditionsAndBlocks ( generator, node, _name, i = 0 ) {

export default {
enter ( generator, node ) {
const { params } = generator.current;
const params = generator.current.params.join( ', ' );
const name = generator.getUniqueName( `ifBlock` );
const getBlock = generator.getUniqueName( `getBlock` );
const currentBlock = generator.getUniqueName( `currentBlock` );
Expand Down
2 changes: 1 addition & 1 deletion src/generators/server-side-rendering/visitors/EachBlock.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ export default {
// TODO should this be the generator's job? It's duplicated between
// here and the equivalent DOM compiler visitor
const contexts = Object.assign( {}, generator.current.contexts );
contexts[ node.context ] = true;
contexts[ node.context ] = node.context;

const indexes = Object.assign( {}, generator.current.indexes );
if ( node.index ) indexes[ node.index ] = node.context;
Expand Down
9 changes: 9 additions & 0 deletions test/generator/deconflict-contexts/_config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
export default {
html: `
<ul><li>foo</li><li>bar</li><li>baz</li></ul>
`,

data: {
components: [ 'foo', 'bar', 'baz' ]
}
};
5 changes: 5 additions & 0 deletions test/generator/deconflict-contexts/main.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<ul>
{{#each components as component}}
<li>{{component}}</li>
{{/each}}
</ul>

0 comments on commit c3ebf25

Please sign in to comment.