From e25d4e752c0f7063e9fc9e5d19e4711d28adf872 Mon Sep 17 00:00:00 2001 From: Conduitry Date: Fri, 17 Mar 2017 20:11:32 -0400 Subject: [PATCH 01/10] deconflict non-helper functions and variables (#388) --- src/generators/dom/index.js | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/generators/dom/index.js b/src/generators/dom/index.js index 406e676e95..5ed7c5b8b2 100644 --- a/src/generators/dom/index.js +++ b/src/generators/dom/index.js @@ -131,12 +131,14 @@ class DomGenerator extends Generator { node.children = []; } - helper ( name ) { + helper ( name, isSharedHelper ) { if ( this.options.dev && `${name}Dev` in shared ) { name = `${name}Dev`; } - this.uses[ name ] = true; + if ( isSharedHelper !== false ) { + this.uses[ name ] = true; + } if ( !( name in this.aliases ) ) { let alias = name; @@ -188,7 +190,7 @@ export default function dom ( parsed, source, options, names ) { } generator.push({ - name: 'renderMainFragment', + name: generator.helper( 'renderMainFragment', false ), namespace, target: 'target', localElementDepth: 0, @@ -238,12 +240,12 @@ export default function dom ( parsed, source, options, names ) { }); builders.main.addBlock( deindent` - function applyComputations ( state, newState, oldState, isInitial ) { + function ${generator.helper( 'applyComputations', false )} ( state, newState, oldState, isInitial ) { ${builder} } ` ); - builders._set.addLine( `applyComputations( this._state, newState, oldState, false )` ); + builders._set.addLine( `${generator.helper( 'applyComputations', false )}( this._state, newState, oldState, false )` ); } // TODO is the `if` necessary? @@ -259,13 +261,13 @@ export default function dom ( parsed, source, options, names ) { if ( parsed.css && options.css !== false ) { builders.main.addBlock( deindent` - var addedCss = false; - function addCss () { + var ${generator.helper( 'addedCss', false )} = false; + function ${generator.helper( 'addCss', false )} () { var style = ${generator.helper( 'createElement' )}( 'style' ); style.textContent = ${JSON.stringify( processCss( parsed, generator.code ) )}; ${generator.helper( 'appendNode' )}( style, document.head ); - addedCss = true; + ${generator.helper( 'addedCss', false )} = true; } ` ); } @@ -276,7 +278,7 @@ export default function dom ( parsed, source, options, names ) { builders.init.addLine( `this._torndown = false;` ); if ( parsed.css && options.css !== false ) { - builders.init.addLine( `if ( !addedCss ) addCss();` ); + builders.init.addLine( `if ( !${generator.helper( 'addedCss', false )} ) ${generator.helper( 'addCss', false )}();` ); } if ( generator.hasComponents ) { @@ -286,7 +288,7 @@ export default function dom ( parsed, source, options, names ) { if ( generator.hasComplexBindings ) { builders.init.addBlock( deindent` this._bindings = []; - this._fragment = renderMainFragment( this._state, this ); + this._fragment = ${generator.helper( 'renderMainFragment', false )}( this._state, this ); if ( options.target ) this._fragment.mount( options.target, null ); while ( this._bindings.length ) this._bindings.pop()(); ` ); @@ -294,7 +296,7 @@ export default function dom ( parsed, source, options, names ) { builders._set.addLine( `while ( this._bindings.length ) this._bindings.pop()();` ); } else { builders.init.addBlock( deindent` - this._fragment = renderMainFragment( this._state, this ); + this._fragment = ${generator.helper( 'renderMainFragment', false )}( this._state, this ); if ( options.target ) this._fragment.mount( options.target, null ); ` ); } @@ -327,7 +329,7 @@ export default function dom ( parsed, source, options, names ) { if ( templateProperties.computed ) { constructorBlock.addLine( - `applyComputations( this._state, this._state, {}, true );` + `${generator.helper( 'applyComputations', false )}( this._state, this._state, {}, true );` ); } @@ -419,4 +421,4 @@ export default function dom ( parsed, source, options, names ) { } return generator.generate( builders.main.toString(), options, { name, format } ); -} \ No newline at end of file +} From 331a1a2bb2e5f2c4ff00e82179cce22a880856a4 Mon Sep 17 00:00:00 2001 From: Conduitry Date: Fri, 17 Mar 2017 20:28:56 -0400 Subject: [PATCH 02/10] nicer way of generating aliases for non-helper functions --- src/generators/dom/index.js | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/src/generators/dom/index.js b/src/generators/dom/index.js index 5ed7c5b8b2..f3b9d00ae6 100644 --- a/src/generators/dom/index.js +++ b/src/generators/dom/index.js @@ -131,15 +131,17 @@ class DomGenerator extends Generator { node.children = []; } - helper ( name, isSharedHelper ) { + helper ( name ) { if ( this.options.dev && `${name}Dev` in shared ) { name = `${name}Dev`; } - if ( isSharedHelper !== false ) { - this.uses[ name ] = true; - } + this.uses[ name ] = true; + + return this.alias( name ); + } + alias ( name ) { if ( !( name in this.aliases ) ) { let alias = name; let i = 1; @@ -190,7 +192,7 @@ export default function dom ( parsed, source, options, names ) { } generator.push({ - name: generator.helper( 'renderMainFragment', false ), + name: generator.alias( 'renderMainFragment' ), namespace, target: 'target', localElementDepth: 0, @@ -240,12 +242,12 @@ export default function dom ( parsed, source, options, names ) { }); builders.main.addBlock( deindent` - function ${generator.helper( 'applyComputations', false )} ( state, newState, oldState, isInitial ) { + function ${generator.alias( 'applyComputations' )} ( state, newState, oldState, isInitial ) { ${builder} } ` ); - builders._set.addLine( `${generator.helper( 'applyComputations', false )}( this._state, newState, oldState, false )` ); + builders._set.addLine( `${generator.alias( 'applyComputations' )}( this._state, newState, oldState, false )` ); } // TODO is the `if` necessary? @@ -261,13 +263,13 @@ export default function dom ( parsed, source, options, names ) { if ( parsed.css && options.css !== false ) { builders.main.addBlock( deindent` - var ${generator.helper( 'addedCss', false )} = false; - function ${generator.helper( 'addCss', false )} () { + var ${generator.alias( 'addedCss' )} = false; + function ${generator.alias( 'addCss' )} () { var style = ${generator.helper( 'createElement' )}( 'style' ); style.textContent = ${JSON.stringify( processCss( parsed, generator.code ) )}; ${generator.helper( 'appendNode' )}( style, document.head ); - ${generator.helper( 'addedCss', false )} = true; + ${generator.alias( 'addedCss' )} = true; } ` ); } @@ -278,7 +280,7 @@ export default function dom ( parsed, source, options, names ) { builders.init.addLine( `this._torndown = false;` ); if ( parsed.css && options.css !== false ) { - builders.init.addLine( `if ( !${generator.helper( 'addedCss', false )} ) ${generator.helper( 'addCss', false )}();` ); + builders.init.addLine( `if ( !${generator.alias( 'addedCss' )} ) ${generator.alias( 'addCss' )}();` ); } if ( generator.hasComponents ) { @@ -288,7 +290,7 @@ export default function dom ( parsed, source, options, names ) { if ( generator.hasComplexBindings ) { builders.init.addBlock( deindent` this._bindings = []; - this._fragment = ${generator.helper( 'renderMainFragment', false )}( this._state, this ); + this._fragment = ${generator.alas( 'renderMainFragment' )}( this._state, this ); if ( options.target ) this._fragment.mount( options.target, null ); while ( this._bindings.length ) this._bindings.pop()(); ` ); @@ -296,7 +298,7 @@ export default function dom ( parsed, source, options, names ) { builders._set.addLine( `while ( this._bindings.length ) this._bindings.pop()();` ); } else { builders.init.addBlock( deindent` - this._fragment = ${generator.helper( 'renderMainFragment', false )}( this._state, this ); + this._fragment = ${generator.alias( 'renderMainFragment' )}( this._state, this ); if ( options.target ) this._fragment.mount( options.target, null ); ` ); } @@ -329,7 +331,7 @@ export default function dom ( parsed, source, options, names ) { if ( templateProperties.computed ) { constructorBlock.addLine( - `${generator.helper( 'applyComputations', false )}( this._state, this._state, {}, true );` + `${generator.alias( 'applyComputations' )}( this._state, this._state, {}, true );` ); } From 8f6499a9dd65c4b460c44c848dec38223dc012eb Mon Sep 17 00:00:00 2001 From: Conduitry Date: Fri, 17 Mar 2017 20:32:15 -0400 Subject: [PATCH 03/10] alas poor typo --- src/generators/dom/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/generators/dom/index.js b/src/generators/dom/index.js index f3b9d00ae6..2872a948c1 100644 --- a/src/generators/dom/index.js +++ b/src/generators/dom/index.js @@ -290,7 +290,7 @@ export default function dom ( parsed, source, options, names ) { if ( generator.hasComplexBindings ) { builders.init.addBlock( deindent` this._bindings = []; - this._fragment = ${generator.alas( 'renderMainFragment' )}( this._state, this ); + this._fragment = ${generator.alias( 'renderMainFragment' )}( this._state, this ); if ( options.target ) this._fragment.mount( options.target, null ); while ( this._bindings.length ) this._bindings.pop()(); ` ); From dfd73987e348f6659022b6de8e015944c5f9e4cd Mon Sep 17 00:00:00 2001 From: Conduitry Date: Fri, 17 Mar 2017 20:55:10 -0400 Subject: [PATCH 04/10] add unit test --- .../samples/deconflict-non-helpers/_config.js | 8 ++++++++ .../samples/deconflict-non-helpers/main.html | 17 +++++++++++++++++ .../samples/deconflict-non-helpers/module.js | 4 ++++ 3 files changed, 29 insertions(+) create mode 100644 test/generator/samples/deconflict-non-helpers/_config.js create mode 100644 test/generator/samples/deconflict-non-helpers/main.html create mode 100644 test/generator/samples/deconflict-non-helpers/module.js diff --git a/test/generator/samples/deconflict-non-helpers/_config.js b/test/generator/samples/deconflict-non-helpers/_config.js new file mode 100644 index 0000000000..978e4c27e3 --- /dev/null +++ b/test/generator/samples/deconflict-non-helpers/_config.js @@ -0,0 +1,8 @@ +export default { + html: `ABCD`, + + test ( assert, component ) { + assert.equal( component.get( 'compute' ), 'ABCD' ); + component.destroy(); + } +}; diff --git a/test/generator/samples/deconflict-non-helpers/main.html b/test/generator/samples/deconflict-non-helpers/main.html new file mode 100644 index 0000000000..8541b090f9 --- /dev/null +++ b/test/generator/samples/deconflict-non-helpers/main.html @@ -0,0 +1,17 @@ +{{compute}} + + diff --git a/test/generator/samples/deconflict-non-helpers/module.js b/test/generator/samples/deconflict-non-helpers/module.js new file mode 100644 index 0000000000..ff4e71e251 --- /dev/null +++ b/test/generator/samples/deconflict-non-helpers/module.js @@ -0,0 +1,4 @@ +export const addCss = 'a'; +export const addedCss = 'b'; +export const applyComputations = 'c'; +export const renderMainFragment = 'd'; From 3070abc291d1a0315c67191c83e21546371d9733 Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sat, 18 Mar 2017 09:01:26 -0400 Subject: [PATCH 05/10] -> v1.12.1 --- CHANGELOG.md | 5 +++++ package.json | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ae2ebd57f..a45719c283 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Svelte changelog +## 1.12.1 + +* Deconflict non-helper functions (`addCss` etc) ([#388](https://github.com/sveltejs/svelte/issues/388)) +* Allow reserved words in tags, e.g. `{{class}}` ([#383](https://github.com/sveltejs/svelte/issues/383)) + ## 1.12.0 * Shorthand attributes — `` is equivalent to `` ([#384](https://github.com/sveltejs/svelte/pull/384)) diff --git a/package.json b/package.json index d2071c6e29..91073752a7 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "svelte", - "version": "1.12.0", + "version": "1.12.1", "description": "The magical disappearing UI framework", "main": "compiler/svelte.js", "files": [ From cede99d68bb27c8ede641f113a9cc877f17013c0 Mon Sep 17 00:00:00 2001 From: Conduitry Date: Fri, 17 Mar 2017 23:22:40 -0400 Subject: [PATCH 06/10] use direct references to components that have been imported --- src/generators/dom/index.js | 11 +++++++++++ src/generators/dom/visitors/Component.js | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/generators/dom/index.js b/src/generators/dom/index.js index 2872a948c1..e066a6c48e 100644 --- a/src/generators/dom/index.js +++ b/src/generators/dom/index.js @@ -191,6 +191,17 @@ export default function dom ( parsed, source, options, names ) { // TODO remove the namespace property from the generated code, it's unused past this point } + if ( templateProperties.components ) { + generator.importedComponents = {}; + templateProperties.components.value.properties.forEach( property => { + const key = property.key.name; + const value = source.slice( property.value.start, property.value.end ); + if ( generator.importedNames[ value ] ) { + generator.importedComponents[ key ] = value; + } + }); + } + generator.push({ name: generator.alias( 'renderMainFragment' ), namespace, diff --git a/src/generators/dom/visitors/Component.js b/src/generators/dom/visitors/Component.js index 990b947f7c..7165a8a7c2 100644 --- a/src/generators/dom/visitors/Component.js +++ b/src/generators/dom/visitors/Component.js @@ -106,7 +106,7 @@ export default { componentInitProperties.push(`data: ${name}_initialData`); } - const expression = node.name === ':Self' ? generator.name : `template.components.${node.name}`; + const expression = node.name === ':Self' ? generator.name : generator.importedComponents[ node.name ] || `template.components.${node.name}`; local.init.addBlockAtStart( deindent` ${statements.join( '\n\n' )} From 77321f8356e4db3696fd59c147175c8b94f73564 Mon Sep 17 00:00:00 2001 From: Conduitry Date: Sat, 18 Mar 2017 15:44:32 -0400 Subject: [PATCH 07/10] remove `namespace` key from generated code --- src/generators/Generator.js | 4 +++- src/generators/dom/index.js | 5 +++-- src/utils/removeObjectKey.js | 9 +++++++++ 3 files changed, 15 insertions(+), 3 deletions(-) create mode 100644 src/utils/removeObjectKey.js diff --git a/src/generators/Generator.js b/src/generators/Generator.js index f0571d96c1..469b271f85 100644 --- a/src/generators/Generator.js +++ b/src/generators/Generator.js @@ -232,6 +232,7 @@ export default class Generator { const imports = this.imports; const computations = []; + let defaultExport = null; const templateProperties = {}; if ( js ) { @@ -251,7 +252,7 @@ export default class Generator { } } - const defaultExport = js.content.body.find( node => node.type === 'ExportDefaultDeclaration' ); + defaultExport = js.content.body.find( node => node.type === 'ExportDefaultDeclaration' ); if ( defaultExport ) { const finalNode = js.content.body[ js.content.body.length - 1 ]; @@ -319,6 +320,7 @@ export default class Generator { return { computations, + defaultExport, templateProperties }; } diff --git a/src/generators/dom/index.js b/src/generators/dom/index.js index e066a6c48e..0a4036c355 100644 --- a/src/generators/dom/index.js +++ b/src/generators/dom/index.js @@ -3,6 +3,7 @@ import getBuilders from './utils/getBuilders.js'; import CodeBuilder from '../../utils/CodeBuilder.js'; import namespaces from '../../utils/namespaces.js'; import processCss from '../shared/processCss.js'; +import removeObjectKey from '../../utils/removeObjectKey.js'; import visitors from './visitors/index.js'; import Generator from '../Generator.js'; import * as shared from '../../shared/index.js'; @@ -162,7 +163,7 @@ export default function dom ( parsed, source, options, names ) { const generator = new DomGenerator( parsed, source, name, names, visitors, options ); - const { computations, templateProperties } = generator.parseJs(); + const { computations, defaultExport, templateProperties } = generator.parseJs(); // Remove these after version 2 if ( templateProperties.onrender ) { @@ -188,7 +189,7 @@ export default function dom ( parsed, source, options, names ) { const ns = templateProperties.namespace.value.value; namespace = namespaces[ ns ] || ns; - // TODO remove the namespace property from the generated code, it's unused past this point + removeObjectKey( generator.code, defaultExport.declaration, 'namespace' ); } if ( templateProperties.components ) { diff --git a/src/utils/removeObjectKey.js b/src/utils/removeObjectKey.js new file mode 100644 index 0000000000..7399a6a2bb --- /dev/null +++ b/src/utils/removeObjectKey.js @@ -0,0 +1,9 @@ +export default function removeObjectKey ( code, parsed, key ) { + if ( parsed.type !== 'ObjectExpression' ) return; + const properties = parsed.properties; + const index = properties.findIndex( property => property.key.type === 'Identifier' && property.key.name === key ); + if ( index === -1 ) return; + const a = properties[ index ].start; + const b = index < properties.length - 1 ? properties[ index + 1 ].start : properties[ index ].end; + code.remove( a, b ); +} From 25a26613b3f11e244b18c26e8f6789d9bfd3a592 Mon Sep 17 00:00:00 2001 From: Conduitry Date: Sat, 18 Mar 2017 17:39:30 -0400 Subject: [PATCH 08/10] remove the component references in the export that we will be referring to directly --- src/generators/dom/index.js | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/generators/dom/index.js b/src/generators/dom/index.js index 0a4036c355..91c1085082 100644 --- a/src/generators/dom/index.js +++ b/src/generators/dom/index.js @@ -18,6 +18,8 @@ class DomGenerator extends Generator { // Svelte's builtin `import { get, ... } from 'svelte/shared.js'`; this.importedNames = {}; this.aliases = {}; + + this.importedComponents = {}; } addElement ( name, renderStatement, needsIdentifier = false ) { @@ -193,14 +195,25 @@ export default function dom ( parsed, source, options, names ) { } if ( templateProperties.components ) { - generator.importedComponents = {}; + let hasNonImportedComponent = false; templateProperties.components.value.properties.forEach( property => { const key = property.key.name; const value = source.slice( property.value.start, property.value.end ); if ( generator.importedNames[ value ] ) { generator.importedComponents[ key ] = value; + } else { + hasNonImportedComponent = true; } }); + if ( hasNonImportedComponent ) { + // remove the specific components which were imported, as we'll refer to them directly + Object.keys( generator.importedComponents ).forEach ( key => { + removeObjectKey( generator.code, templateProperties.components.value, key ); + }); + } else { + // remove the entire components portion of the export + removeObjectKey( generator.code, defaultExport.declaration, 'components' ); + } } generator.push({ From 8e5c7ed21ebabdc9b7d1a80632a2c8c1b3fcb4e9 Mon Sep 17 00:00:00 2001 From: Conduitry Date: Sat, 18 Mar 2017 17:51:57 -0400 Subject: [PATCH 09/10] removeObjectKey API tweak --- src/generators/dom/index.js | 6 +++--- src/utils/removeObjectKey.js | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/generators/dom/index.js b/src/generators/dom/index.js index 91c1085082..ebf478f5a2 100644 --- a/src/generators/dom/index.js +++ b/src/generators/dom/index.js @@ -191,7 +191,7 @@ export default function dom ( parsed, source, options, names ) { const ns = templateProperties.namespace.value.value; namespace = namespaces[ ns ] || ns; - removeObjectKey( generator.code, defaultExport.declaration, 'namespace' ); + removeObjectKey( generator, defaultExport.declaration, 'namespace' ); } if ( templateProperties.components ) { @@ -208,11 +208,11 @@ export default function dom ( parsed, source, options, names ) { if ( hasNonImportedComponent ) { // remove the specific components which were imported, as we'll refer to them directly Object.keys( generator.importedComponents ).forEach ( key => { - removeObjectKey( generator.code, templateProperties.components.value, key ); + removeObjectKey( generator, templateProperties.components.value, key ); }); } else { // remove the entire components portion of the export - removeObjectKey( generator.code, defaultExport.declaration, 'components' ); + removeObjectKey( generator, defaultExport.declaration, 'components' ); } } diff --git a/src/utils/removeObjectKey.js b/src/utils/removeObjectKey.js index 7399a6a2bb..cbd75eb1f5 100644 --- a/src/utils/removeObjectKey.js +++ b/src/utils/removeObjectKey.js @@ -1,9 +1,9 @@ -export default function removeObjectKey ( code, parsed, key ) { - if ( parsed.type !== 'ObjectExpression' ) return; - const properties = parsed.properties; +export default function removeObjectKey ( generator, node, key ) { + if ( node.type !== 'ObjectExpression' ) return; + const properties = node.properties; const index = properties.findIndex( property => property.key.type === 'Identifier' && property.key.name === key ); if ( index === -1 ) return; const a = properties[ index ].start; const b = index < properties.length - 1 ? properties[ index + 1 ].start : properties[ index ].end; - code.remove( a, b ); + generator.code.remove( a, b ); } From d2dce30cdce956ac00ac0647f012113204077310 Mon Sep 17 00:00:00 2001 From: Conduitry Date: Sat, 18 Mar 2017 18:11:05 -0400 Subject: [PATCH 10/10] add unit test for imported components that are then renamed as they're used --- .../imported-renamed-components/ComponentOne.html | 1 + .../imported-renamed-components/ComponentTwo.html | 1 + .../samples/imported-renamed-components/_config.js | 3 +++ .../samples/imported-renamed-components/main.html | 10 ++++++++++ 4 files changed, 15 insertions(+) create mode 100644 test/generator/samples/imported-renamed-components/ComponentOne.html create mode 100644 test/generator/samples/imported-renamed-components/ComponentTwo.html create mode 100644 test/generator/samples/imported-renamed-components/_config.js create mode 100644 test/generator/samples/imported-renamed-components/main.html diff --git a/test/generator/samples/imported-renamed-components/ComponentOne.html b/test/generator/samples/imported-renamed-components/ComponentOne.html new file mode 100644 index 0000000000..3609f20ebd --- /dev/null +++ b/test/generator/samples/imported-renamed-components/ComponentOne.html @@ -0,0 +1 @@ +One diff --git a/test/generator/samples/imported-renamed-components/ComponentTwo.html b/test/generator/samples/imported-renamed-components/ComponentTwo.html new file mode 100644 index 0000000000..3b0086f91c --- /dev/null +++ b/test/generator/samples/imported-renamed-components/ComponentTwo.html @@ -0,0 +1 @@ +Two diff --git a/test/generator/samples/imported-renamed-components/_config.js b/test/generator/samples/imported-renamed-components/_config.js new file mode 100644 index 0000000000..9a9e325b84 --- /dev/null +++ b/test/generator/samples/imported-renamed-components/_config.js @@ -0,0 +1,3 @@ +export default { + html: `OneTwo` +}; diff --git a/test/generator/samples/imported-renamed-components/main.html b/test/generator/samples/imported-renamed-components/main.html new file mode 100644 index 0000000000..7c25eaceab --- /dev/null +++ b/test/generator/samples/imported-renamed-components/main.html @@ -0,0 +1,10 @@ + + +