From 99d6502bfd2397b58c55c4e99034efec955b4e8e Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 10 Apr 2017 14:20:50 -0400 Subject: [PATCH 1/4] better error for attempts to use getters/setters for methods. closes #425 --- src/validate/js/propValidators/methods.js | 2 ++ src/validate/js/utils/checkForAccessors.js | 7 +++++++ test/validator/index.js | 5 +++-- .../properties-methods-getters-setters/errors.json | 8 ++++++++ .../properties-methods-getters-setters/input.html | 9 +++++++++ 5 files changed, 29 insertions(+), 2 deletions(-) create mode 100644 src/validate/js/utils/checkForAccessors.js create mode 100644 test/validator/samples/properties-methods-getters-setters/errors.json create mode 100644 test/validator/samples/properties-methods-getters-setters/input.html diff --git a/src/validate/js/propValidators/methods.js b/src/validate/js/propValidators/methods.js index 23b10f03a5..0bddc3d184 100644 --- a/src/validate/js/propValidators/methods.js +++ b/src/validate/js/propValidators/methods.js @@ -1,3 +1,4 @@ +import checkForAccessors from '../utils/checkForAccessors.js'; import checkForDupes from '../utils/checkForDupes.js'; import checkForComputedKeys from '../utils/checkForComputedKeys.js'; import usesThisOrArguments from '../utils/usesThisOrArguments.js'; @@ -10,6 +11,7 @@ export default function methods ( validator, prop ) { return; } + checkForAccessors( validator, prop.value.properties, 'Methods' ); checkForDupes( validator, prop.value.properties ); checkForComputedKeys( validator, prop.value.properties ); diff --git a/src/validate/js/utils/checkForAccessors.js b/src/validate/js/utils/checkForAccessors.js new file mode 100644 index 0000000000..c6fad53dc8 --- /dev/null +++ b/src/validate/js/utils/checkForAccessors.js @@ -0,0 +1,7 @@ +export default function checkForAccessors ( validator, properties, label ) { + properties.forEach( prop => { + if ( prop.kind !== 'init' ) { + validator.error( `${label} cannot use getters and setters`, prop.start ); + } + }); +} diff --git a/test/validator/index.js b/test/validator/index.js index b3de9046a9..9b88441f34 100644 --- a/test/validator/index.js +++ b/test/validator/index.js @@ -1,12 +1,13 @@ import * as fs from 'fs'; import assert from 'assert'; -import { svelte, exists, tryToLoadJson } from '../helpers.js'; +import { svelte, tryToLoadJson } from '../helpers.js'; describe( 'validate', () => { fs.readdirSync( 'test/validator/samples' ).forEach( dir => { if ( dir[0] === '.' ) return; - const solo = exists( `test/validator/samples/${dir}/solo` ); + // add .solo to a sample directory name to only run that test + const solo = /\.solo/.test( dir ); if ( solo && process.env.CI ) { throw new Error( 'Forgot to remove `solo: true` from test' ); diff --git a/test/validator/samples/properties-methods-getters-setters/errors.json b/test/validator/samples/properties-methods-getters-setters/errors.json new file mode 100644 index 0000000000..40481f12e0 --- /dev/null +++ b/test/validator/samples/properties-methods-getters-setters/errors.json @@ -0,0 +1,8 @@ +[{ + "message": "Methods cannot use getters and setters", + "loc": { + "line": 4, + "column": 3 + }, + "pos": 43 +}] diff --git a/test/validator/samples/properties-methods-getters-setters/input.html b/test/validator/samples/properties-methods-getters-setters/input.html new file mode 100644 index 0000000000..1282aff074 --- /dev/null +++ b/test/validator/samples/properties-methods-getters-setters/input.html @@ -0,0 +1,9 @@ + \ No newline at end of file From 182a04e8fc0a73fede64410613190a781b77ea07 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 10 Apr 2017 14:54:19 -0400 Subject: [PATCH 2/4] dont create whitespace nodes inside elements like + + + + \ No newline at end of file From 2852b96e65fc1ba8c05bb2bce903db1c7102ba3c Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 10 Apr 2017 15:10:02 -0400 Subject: [PATCH 3/4] collapse consecutive if-statements with the same condition (#450) --- src/generators/dom/index.js | 9 +-- src/utils/CodeBuilder.js | 29 +++++++- .../samples/computed-collapsed-if/expected.js | 74 +++++++++++++++++++ .../samples/computed-collapsed-if/input.html | 8 ++ 4 files changed, 114 insertions(+), 6 deletions(-) create mode 100644 test/js/samples/computed-collapsed-if/expected.js create mode 100644 test/js/samples/computed-collapsed-if/input.html diff --git a/src/generators/dom/index.js b/src/generators/dom/index.js index dbb054e7c6..9472538940 100644 --- a/src/generators/dom/index.js +++ b/src/generators/dom/index.js @@ -94,11 +94,10 @@ export default function dom ( parsed, source, options ) { const differs = generator.helper( 'differs' ); computations.forEach( ({ key, deps }) => { - builder.addBlock( deindent` - if ( isInitial || ${deps.map( dep => `( '${dep}' in newState && ${differs}( state.${dep}, oldState.${dep} ) )` ).join( ' || ' )} ) { - state.${key} = newState.${key} = ${generator.alias( 'template' )}.computed.${key}( ${deps.map( dep => `state.${dep}` ).join( ', ' )} ); - } - ` ); + const condition = `isInitial || ${deps.map( dep => `( '${dep}' in newState && ${differs}( state.${dep}, oldState.${dep} ) )` ).join( ' || ' )}`; + const statement = `state.${key} = newState.${key} = ${generator.alias( 'template' )}.computed.${key}( ${deps.map( dep => `state.${dep}` ).join( ', ' )} );` + + builder.addConditionalLine( condition, statement ); }); builders.main.addBlock( deindent` diff --git a/src/utils/CodeBuilder.js b/src/utils/CodeBuilder.js index 1caaf9ff84..357bf85af8 100644 --- a/src/utils/CodeBuilder.js +++ b/src/utils/CodeBuilder.js @@ -7,9 +7,31 @@ export default class CodeBuilder { this.first = null; this.last = null; + + this.lastCondition = null; + } + + addConditionalLine ( condition, line ) { + if ( condition === this.lastCondition ) { + this.result += `\n\t${line}`; + } else { + if ( this.lastCondition ) { + this.result += `\n}`; + } + + this.result += `if ( ${condition} ) {\n\t${line}`; + this.lastCondition = condition; + } + + this.last = BLOCK; } addLine ( line ) { + if ( this.lastCondition ) { + this.result += `\n}`; + this.lastCondition = null; + } + if ( this.last === BLOCK ) { this.result += `\n\n${line}`; } else if ( this.last === LINE ) { @@ -36,6 +58,11 @@ export default class CodeBuilder { } addBlock ( block ) { + if ( this.lastCondition ) { + this.result += `\n}`; + this.lastCondition = null; + } + if ( this.result ) { this.result += `\n\n${block}`; } else { @@ -62,6 +89,6 @@ export default class CodeBuilder { } toString () { - return this.result.trim(); + return this.result.trim() + ( this.lastCondition ? `\n}` : `` ); } } diff --git a/test/js/samples/computed-collapsed-if/expected.js b/test/js/samples/computed-collapsed-if/expected.js new file mode 100644 index 0000000000..d8792a974f --- /dev/null +++ b/test/js/samples/computed-collapsed-if/expected.js @@ -0,0 +1,74 @@ +import { assign, differs, dispatchObservers, noop, proto } from "svelte/shared.js"; + +function recompute ( state, newState, oldState, isInitial ) { + if ( isInitial || ( 'x' in newState && differs( state.x, oldState.x ) ) ) { + state.a = newState.a = template.computed.a( state.x ); + state.b = newState.b = template.computed.b( state.x ); + } +} + +var template = (function () { + return { + computed: { + a: x => x * 2, + b: x => x * 3 + } + }; +}()); + +function create_main_fragment ( root, component ) { + + + return { + mount: noop, + + update: noop, + + destroy: noop + }; +} + +function SvelteComponent ( options ) { + options = options || {}; + this._state = options.data || {}; + recompute( this._state, this._state, {}, true ); + + this._observers = { + pre: Object.create( null ), + post: Object.create( null ) + }; + + this._handlers = Object.create( null ); + + this._root = options._root; + this._yield = options._yield; + + this._torndown = false; + + this._fragment = create_main_fragment( this._state, this ); + if ( options.target ) this._fragment.mount( options.target, null ); +} + +assign( SvelteComponent.prototype, proto ); + +SvelteComponent.prototype._set = function _set ( newState ) { + var oldState = this._state; + this._state = assign( {}, oldState, newState ); + recompute( this._state, newState, oldState, false ) + + dispatchObservers( this, this._observers.pre, newState, oldState ); + if ( this._fragment ) this._fragment.update( newState, this._state ); + dispatchObservers( this, this._observers.post, newState, oldState ); +}; + +SvelteComponent.prototype.teardown = SvelteComponent.prototype.destroy = function destroy ( detach ) { + this.fire( 'destroy' ); + + this._fragment.destroy( detach !== false ); + this._fragment = null; + + this._state = {}; + this._torndown = true; +}; + +export default SvelteComponent; \ No newline at end of file diff --git a/test/js/samples/computed-collapsed-if/input.html b/test/js/samples/computed-collapsed-if/input.html new file mode 100644 index 0000000000..a68513b860 --- /dev/null +++ b/test/js/samples/computed-collapsed-if/input.html @@ -0,0 +1,8 @@ + \ No newline at end of file From b096385df18210b96dfd62576b0aad0eafac129d Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 10 Apr 2017 15:12:16 -0400 Subject: [PATCH 4/4] lint --- 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 9472538940..ddf3f39cab 100644 --- a/src/generators/dom/index.js +++ b/src/generators/dom/index.js @@ -95,7 +95,7 @@ export default function dom ( parsed, source, options ) { computations.forEach( ({ key, deps }) => { const condition = `isInitial || ${deps.map( dep => `( '${dep}' in newState && ${differs}( state.${dep}, oldState.${dep} ) )` ).join( ' || ' )}`; - const statement = `state.${key} = newState.${key} = ${generator.alias( 'template' )}.computed.${key}( ${deps.map( dep => `state.${dep}` ).join( ', ' )} );` + const statement = `state.${key} = newState.${key} = ${generator.alias( 'template' )}.computed.${key}( ${deps.map( dep => `state.${dep}` ).join( ', ' )} );`; builder.addConditionalLine( condition, statement ); });