Skip to content

Commit

Permalink
dev warnings for bad arguments to component.observe (#369), and compo…
Browse files Browse the repository at this point in the history
…nent.on("teardown") (#365)
  • Loading branch information
Rich-Harris committed Mar 16, 2017
1 parent a801e18 commit 06f89d1
Show file tree
Hide file tree
Showing 7 changed files with 60 additions and 13 deletions.
10 changes: 5 additions & 5 deletions src/generators/dom/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 ) {
Expand Down Expand Up @@ -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 ) ) {
Expand Down Expand Up @@ -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};` );
}
});
Expand All @@ -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;
Expand Down
2 changes: 0 additions & 2 deletions src/generators/dom/utils/devHelperLookup.js

This file was deleted.

46 changes: 46 additions & 0 deletions src/shared/methods.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 );

Expand Down
9 changes: 6 additions & 3 deletions test/generator/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'` );
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -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`
]
};
2 changes: 1 addition & 1 deletion test/generator/samples/events-lifecycle/_config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});
Expand Down

0 comments on commit 06f89d1

Please sign in to comment.