From de042daed38a5c222583883f6d0d8508e10b8f59 Mon Sep 17 00:00:00 2001 From: Conduitry Date: Sat, 11 Mar 2017 23:10:49 -0500 Subject: [PATCH 1/4] initial support for dev versions of shared helpers --- src/generators/Generator.js | 3 +- src/generators/dom/index.js | 46 +++++++------------ src/generators/dom/utils/devHelperLookup.js | 2 + src/generators/server-side-rendering/index.js | 6 +-- 4 files changed, 23 insertions(+), 34 deletions(-) create mode 100644 src/generators/dom/utils/devHelperLookup.js diff --git a/src/generators/Generator.js b/src/generators/Generator.js index e25fdc5a9bea..f3aa58dc26be 100644 --- a/src/generators/Generator.js +++ b/src/generators/Generator.js @@ -9,12 +9,13 @@ import getOutro from './shared/utils/getOutro.js'; import annotateWithScopes from './annotateWithScopes.js'; export default class Generator { - constructor ( parsed, source, name, names, visitors ) { + constructor ( parsed, source, name, names, visitors, options ) { this.parsed = parsed; this.source = source; this.name = name; this.names = names; this.visitors = visitors; + this.options = options; this.imports = []; this.helpers = {}; diff --git a/src/generators/dom/index.js b/src/generators/dom/index.js index 51067279ac22..c6c277bb594a 100644 --- a/src/generators/dom/index.js +++ b/src/generators/dom/index.js @@ -6,10 +6,11 @@ import processCss from '../shared/processCss.js'; import visitors from './visitors/index.js'; import Generator from '../Generator.js'; import * as shared from '../../shared/index.js'; +import devHelperLookup from './utils/devHelperLookup.js'; class DomGenerator extends Generator { - constructor ( parsed, source, name, names, visitors ) { - super( parsed, source, name, names, visitors ); + constructor ( parsed, source, name, names, visitors, options ) { + super( parsed, source, name, names, visitors, options ); this.renderers = []; this.uses = {}; @@ -132,6 +133,9 @@ class DomGenerator extends Generator { } helper ( name ) { + if ( this.options.dev && devHelperLookup[ name ] ) { + name = devHelperLookup[ name ]; + } this.uses[ name ] = true; if ( !( name in this.aliases ) ) { @@ -152,7 +156,7 @@ export default function dom ( parsed, source, options, names ) { const format = options.format || 'es'; const name = options.name || 'SvelteComponent'; - const generator = new DomGenerator( parsed, source, name, names, visitors ); + const generator = new DomGenerator( parsed, source, name, names, visitors, options ); const { computations, templateProperties } = generator.parseJs(); @@ -362,28 +366,14 @@ export default function dom ( parsed, source, options, names ) { const sharedPath = options.shared === true ? 'svelte/shared.js' : options.shared; - builders.main.addBlock( sharedPath ? - deindent` - ${name}.prototype.get = ${generator.helper( 'get' )}; - ${name}.prototype.fire = ${generator.helper( 'fire' )}; - ${name}.prototype.observe = ${generator.helper( 'observe' )}; - ${name}.prototype.on = ${generator.helper( 'on' )}; - ${name}.prototype.set = ${generator.helper( 'set' )}; - ${name}.prototype._flush = ${generator.helper( '_flush' )}; - ` : - deindent` - ${name}.prototype.get = ${shared.get}; - - ${name}.prototype.fire = ${shared.fire}; - - ${name}.prototype.observe = ${shared.observe}; - - ${name}.prototype.on = ${shared.on}; - - ${name}.prototype.set = ${shared.set}; - - ${name}.prototype._flush = ${shared._flush}; - ` ); + [ 'get', 'fire', 'observe', 'on', 'set', '_flush' ].forEach( methodName => { + if ( sharedPath ) { + builders.main.addLine( `${name}.prototype.${methodName} = ${generator.helper( methodName )};` ); + } else { + const fn = shared[ options.dev && devHelperLookup[ methodName ] ? devHelperLookup[ methodName ] : methodName ]; // eslint-disable-line import/namespace + builders.main.addLine( `${name}.prototype.${methodName} = ${fn};` ); + } + }); // TODO deprecate component.teardown() builders.main.addBlock( deindent` @@ -417,11 +407,7 @@ export default function dom ( parsed, source, options, names ) { } else { Object.keys( generator.uses ).forEach( key => { const fn = shared[ key ]; // eslint-disable-line import/namespace - if ( key !== generator.aliases[ key ] ) { - builders.main.addBlock( `var ${generator.aliases[ key ]} = ${fn.toString()}}` ); - } else { - builders.main.addBlock( fn.toString() ); - } + builders.main.addBlock( fn.toString().replace( /^function [^(]*/, 'function ' + generator.aliases[ key ] ) ); }); } diff --git a/src/generators/dom/utils/devHelperLookup.js b/src/generators/dom/utils/devHelperLookup.js new file mode 100644 index 000000000000..02a61bed5e1e --- /dev/null +++ b/src/generators/dom/utils/devHelperLookup.js @@ -0,0 +1,2 @@ +export default { +}; diff --git a/src/generators/server-side-rendering/index.js b/src/generators/server-side-rendering/index.js index 0ff75221fd93..285fb3cca6dc 100644 --- a/src/generators/server-side-rendering/index.js +++ b/src/generators/server-side-rendering/index.js @@ -5,8 +5,8 @@ import visitors from './visitors/index.js'; import Generator from '../Generator.js'; class SsrGenerator extends Generator { - constructor ( parsed, source, name, names, visitors ) { - super( parsed, source, name, names, visitors ); + constructor ( parsed, source, name, names, visitors, options ) { + super( parsed, source, name, names, visitors, options ); this.bindings = []; this.renderCode = ''; } @@ -36,7 +36,7 @@ export default function ssr ( parsed, source, options, names ) { const format = options.format || 'cjs'; const name = options.name || 'SvelteComponent'; - const generator = new SsrGenerator( parsed, source, name, names, visitors ); + const generator = new SsrGenerator( parsed, source, name, names, visitors, options ); const { computations, templateProperties } = generator.parseJs(); From a801e1843b62f7dae8f6f6c03da61f3275baadaa Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 16 Mar 2017 13:53:20 -0400 Subject: [PATCH 2/4] tests for #365 and #369 --- test/generator/index.js | 12 ++++++++++++ .../dev-warning-bad-observe-arguments/_config.js | 7 +++++++ .../dev-warning-bad-observe-arguments/main.html | 9 +++++++++ .../dev-warning-destroy-not-teardown/_config.js | 7 +++++++ .../dev-warning-destroy-not-teardown/main.html | 9 +++++++++ 5 files changed, 44 insertions(+) create mode 100644 test/generator/samples/dev-warning-bad-observe-arguments/_config.js create mode 100644 test/generator/samples/dev-warning-bad-observe-arguments/main.html create mode 100644 test/generator/samples/dev-warning-destroy-not-teardown/_config.js create mode 100644 test/generator/samples/dev-warning-destroy-not-teardown/main.html diff --git a/test/generator/index.js b/test/generator/index.js index 2f647bec1a72..2588129cca99 100644 --- a/test/generator/index.js +++ b/test/generator/index.js @@ -96,6 +96,11 @@ describe( 'generate', () => { // Put the constructor on window for testing window.SvelteComponent = SvelteComponent; + const warnings = []; + window.console.warn = warning => { + warnings.push( warning ); + }; + const target = window.document.querySelector( 'main' ); const component = new SvelteComponent({ @@ -108,6 +113,13 @@ describe( 'generate', () => { throw new Error( 'Expected a runtime error' ); } + if ( config.warnings ) { + assert.deepEqual( warnings, config.warnings ); + } else if ( warnings.length ) { + unintendedError = true; + throw new Error( 'Received unexpected warnings' ); + } + if ( config.html ) { assert.htmlEqual( target.innerHTML, config.html ); } diff --git a/test/generator/samples/dev-warning-bad-observe-arguments/_config.js b/test/generator/samples/dev-warning-bad-observe-arguments/_config.js new file mode 100644 index 000000000000..883b53062b04 --- /dev/null +++ b/test/generator/samples/dev-warning-bad-observe-arguments/_config.js @@ -0,0 +1,7 @@ +export default { + dev: true, + + error ( assert, err ) { + assert.equal( err.message, `The fisrt argument to component.observe(...) must be the name of a top-level property, i.e. 'nested' rather than 'nested.data'` ); + } +}; \ No newline at end of file diff --git a/test/generator/samples/dev-warning-bad-observe-arguments/main.html b/test/generator/samples/dev-warning-bad-observe-arguments/main.html new file mode 100644 index 000000000000..e15ae99fa4d7 --- /dev/null +++ b/test/generator/samples/dev-warning-bad-observe-arguments/main.html @@ -0,0 +1,9 @@ + \ No newline at end of file diff --git a/test/generator/samples/dev-warning-destroy-not-teardown/_config.js b/test/generator/samples/dev-warning-destroy-not-teardown/_config.js new file mode 100644 index 000000000000..fd8577de549e --- /dev/null +++ b/test/generator/samples/dev-warning-destroy-not-teardown/_config.js @@ -0,0 +1,7 @@ +export default { + dev: true, + + warnings: [ + `Use component.on('destroy', ...) instead of component.on('teardown', ...) which has been deprecated` + ] +}; \ No newline at end of file diff --git a/test/generator/samples/dev-warning-destroy-not-teardown/main.html b/test/generator/samples/dev-warning-destroy-not-teardown/main.html new file mode 100644 index 000000000000..22c2e9c28760 --- /dev/null +++ b/test/generator/samples/dev-warning-destroy-not-teardown/main.html @@ -0,0 +1,9 @@ + \ No newline at end of file From 06f89d1d9669a2c2e32a04c9d71ef2392e67de19 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 16 Mar 2017 14:54:48 -0400 Subject: [PATCH 3/4] dev warnings for bad arguments to component.observe (#369), and component.on("teardown") (#365) --- src/generators/dom/index.js | 10 ++-- src/generators/dom/utils/devHelperLookup.js | 2 - src/shared/methods.js | 46 +++++++++++++++++++ test/generator/index.js | 9 ++-- .../_config.js | 2 +- .../_config.js | 2 +- .../samples/events-lifecycle/_config.js | 2 +- 7 files changed, 60 insertions(+), 13 deletions(-) delete mode 100644 src/generators/dom/utils/devHelperLookup.js diff --git a/src/generators/dom/index.js b/src/generators/dom/index.js index 77891d1c300b..86453cd19f61 100644 --- a/src/generators/dom/index.js +++ b/src/generators/dom/index.js @@ -6,7 +6,6 @@ import processCss from '../shared/processCss.js'; import visitors from './visitors/index.js'; import Generator from '../Generator.js'; import * as shared from '../../shared/index.js'; -import devHelperLookup from './utils/devHelperLookup.js'; class DomGenerator extends Generator { constructor ( parsed, source, name, names, visitors, options ) { @@ -133,9 +132,10 @@ class DomGenerator extends Generator { } helper ( name ) { - if ( this.options.dev && devHelperLookup[ name ] ) { - name = devHelperLookup[ name ]; + if ( this.options.dev && `${name}Dev` in shared ) { + name = `${name}Dev`; } + this.uses[ name ] = true; if ( !( name in this.aliases ) ) { @@ -373,7 +373,7 @@ export default function dom ( parsed, source, options, names ) { if ( sharedPath ) { builders.main.addLine( `${name}.prototype.${methodName} = ${generator.helper( methodName )};` ); } else { - const fn = shared[ options.dev && devHelperLookup[ methodName ] ? devHelperLookup[ methodName ] : methodName ]; // eslint-disable-line import/namespace + const fn = shared[ options.dev && `${methodName}Dev` in shared ? `${methodName}Dev` : methodName ]; // eslint-disable-line import/namespace builders.main.addLine( `${name}.prototype.${methodName} = ${fn};` ); } }); @@ -385,7 +385,7 @@ export default function dom ( parsed, source, options, names ) { }; ${name}.prototype.teardown = ${name}.prototype.destroy = function destroy ( detach ) { - this.fire( 'teardown' );${templateProperties.ondestroy ? `\ntemplate.ondestroy.call( this );` : ``} + this.fire( 'destroy' );${templateProperties.ondestroy ? `\ntemplate.ondestroy.call( this );` : ``} this._fragment.teardown( detach !== false ); this._fragment = null; diff --git a/src/generators/dom/utils/devHelperLookup.js b/src/generators/dom/utils/devHelperLookup.js deleted file mode 100644 index 02a61bed5e1e..000000000000 --- a/src/generators/dom/utils/devHelperLookup.js +++ /dev/null @@ -1,2 +0,0 @@ -export default { -}; diff --git a/src/shared/methods.js b/src/shared/methods.js index 34bcdac49817..10b112b803ab 100644 --- a/src/shared/methods.js +++ b/src/shared/methods.js @@ -30,7 +30,53 @@ export function observe ( key, callback, options ) { }; } +export function observeDev ( key, callback, options ) { + var c = ( key = '' + key ).search( /[^\w]/ ); + if ( c > -1 ) { + var message = "The first argument to component.observe(...) must be the name of a top-level property"; + if ( c > 0 ) message += ", i.e. '" + key.slice( 0, c ) + "' rather than '" + key + "'"; + + throw new Error( message ); + } + + var group = ( options && options.defer ) ? this._observers.pre : this._observers.post; + + ( group[ key ] || ( group[ key ] = [] ) ).push( callback ); + + if ( !options || options.init !== false ) { + callback.__calling = true; + callback.call( this, this._state[ key ] ); + callback.__calling = false; + } + + return { + cancel: function () { + var index = group[ key ].indexOf( callback ); + if ( ~index ) group[ key ].splice( index, 1 ); + } + }; +} + export function on ( eventName, handler ) { + if ( eventName === 'teardown' ) return this.on( 'destroy', handler ); + + var handlers = this._handlers[ eventName ] || ( this._handlers[ eventName ] = [] ); + handlers.push( handler ); + + return { + cancel: function () { + var index = handlers.indexOf( handler ); + if ( ~index ) handlers.splice( index, 1 ); + } + }; +} + +export function onDev ( eventName, handler ) { + if ( eventName === 'teardown' ) { + console.warn( "Use component.on('destroy', ...) instead of component.on('teardown', ...) which has been deprecated and will be unsupported in Svelte 2" ); + return this.on( 'destroy', handler ); + } + var handlers = this._handlers[ eventName ] || ( this._handlers[ eventName ] = [] ); handlers.push( handler ); diff --git a/test/generator/index.js b/test/generator/index.js index 2588129cca99..f52831b7c21e 100644 --- a/test/generator/index.js +++ b/test/generator/index.js @@ -96,18 +96,21 @@ describe( 'generate', () => { // Put the constructor on window for testing window.SvelteComponent = SvelteComponent; + const target = window.document.querySelector( 'main' ); + const warnings = []; - window.console.warn = warning => { + const warn = console.warn; + console.warn = warning => { warnings.push( warning ); }; - const target = window.document.querySelector( 'main' ); - const component = new SvelteComponent({ target, data: config.data }); + console.warn = warn; + if ( config.error ) { unintendedError = true; throw new Error( 'Expected a runtime error' ); diff --git a/test/generator/samples/dev-warning-bad-observe-arguments/_config.js b/test/generator/samples/dev-warning-bad-observe-arguments/_config.js index 883b53062b04..0b9183aeba49 100644 --- a/test/generator/samples/dev-warning-bad-observe-arguments/_config.js +++ b/test/generator/samples/dev-warning-bad-observe-arguments/_config.js @@ -2,6 +2,6 @@ export default { dev: true, error ( assert, err ) { - assert.equal( err.message, `The fisrt argument to component.observe(...) must be the name of a top-level property, i.e. 'nested' rather than 'nested.data'` ); + assert.equal( err.message, `The first argument to component.observe(...) must be the name of a top-level property, i.e. 'nested' rather than 'nested.data'` ); } }; \ No newline at end of file diff --git a/test/generator/samples/dev-warning-destroy-not-teardown/_config.js b/test/generator/samples/dev-warning-destroy-not-teardown/_config.js index fd8577de549e..7e9b391f4ba5 100644 --- a/test/generator/samples/dev-warning-destroy-not-teardown/_config.js +++ b/test/generator/samples/dev-warning-destroy-not-teardown/_config.js @@ -2,6 +2,6 @@ export default { dev: true, warnings: [ - `Use component.on('destroy', ...) instead of component.on('teardown', ...) which has been deprecated` + `Use component.on('destroy', ...) instead of component.on('teardown', ...) which has been deprecated and will be unsupported in Svelte 2` ] }; \ No newline at end of file diff --git a/test/generator/samples/events-lifecycle/_config.js b/test/generator/samples/events-lifecycle/_config.js index 3303728f516b..ddd62238402d 100644 --- a/test/generator/samples/events-lifecycle/_config.js +++ b/test/generator/samples/events-lifecycle/_config.js @@ -2,7 +2,7 @@ export default { test ( assert, component ) { let count = 0; - component.on( 'teardown', function () { + component.on( 'destroy', function () { assert.equal( this, component ); count += 1; }); From 6a4c3e46b31e875b9fc8fae304dc98f8872ee80b Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 16 Mar 2017 15:06:02 -0400 Subject: [PATCH 4/4] add shared prototype --- src/generators/dom/index.js | 24 ++++++++++++------------ src/shared/methods.js | 18 ++++++++++++++++++ 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/src/generators/dom/index.js b/src/generators/dom/index.js index 86453cd19f61..3c337a2f2a4a 100644 --- a/src/generators/dom/index.js +++ b/src/generators/dom/index.js @@ -363,20 +363,20 @@ export default function dom ( parsed, source, options, names ) { } ` ); - if ( templateProperties.methods ) { - builders.main.addBlock( `${name}.prototype = template.methods;` ); - } - const sharedPath = options.shared === true ? 'svelte/shared.js' : options.shared; - [ 'get', 'fire', 'observe', 'on', 'set', '_flush' ].forEach( methodName => { - if ( sharedPath ) { - builders.main.addLine( `${name}.prototype.${methodName} = ${generator.helper( methodName )};` ); - } else { - const fn = shared[ options.dev && `${methodName}Dev` in shared ? `${methodName}Dev` : methodName ]; // eslint-disable-line import/namespace - builders.main.addLine( `${name}.prototype.${methodName} = ${fn};` ); + if ( sharedPath ) { + const base = templateProperties.methods ? `{}, template.methods` : `{}`; + builders.main.addBlock( `${name}.prototype = Object.assign( ${base}, ${generator.helper( 'proto' )} );` ); + } else { + if ( templateProperties.methods ) { + builders.main.addBlock( `${name}.prototype = template.methods;` ); } - }); + + [ 'get', 'fire', 'observe', 'on', 'set', '_flush' ].forEach( methodName => { + builders.main.addLine( `${name}.prototype.${methodName} = ${generator.helper( methodName )};` ); + }); + } // TODO deprecate component.teardown() builders.main.addBlock( deindent` @@ -415,4 +415,4 @@ export default function dom ( parsed, source, options, names ) { } return generator.generate( builders.main.toString(), options, { name, format } ); -} +} \ No newline at end of file diff --git a/src/shared/methods.js b/src/shared/methods.js index 10b112b803ab..395420767dde 100644 --- a/src/shared/methods.js +++ b/src/shared/methods.js @@ -101,3 +101,21 @@ export function _flush () { hook.fn.call( hook.context ); } } + +export var proto = { + get: get, + fire: fire, + observe: observe, + on: on, + set: set, + _flush: _flush +}; + +export var protoDev = { + get: get, + fire: fire, + observe: observeDev, + on: onDev, + set: set, + _flush: _flush +}; \ No newline at end of file