Merge pull request #316 from sveltejs/gh-40

[WIP] warn on onrender/onteardown, replace with oncreate/ondestroy
pull/322/head
Rich Harris 9 years ago committed by GitHub
commit 9fc2108fb4

@ -243,7 +243,7 @@ export default class Generator {
}
defaultExport.declaration.properties.forEach( prop => {
templateProperties[ prop.key.name ] = prop.value;
templateProperties[ prop.key.name ] = prop;
});
this.code.prependRight( js.content.start, 'var template = (function () {' );
@ -255,7 +255,7 @@ export default class Generator {
[ 'helpers', 'events', 'components' ].forEach( key => {
if ( templateProperties[ key ] ) {
templateProperties[ key ].properties.forEach( prop => {
templateProperties[ key ].value.properties.forEach( prop => {
this[ key ][ prop.key.name ] = prop.value;
});
}
@ -264,7 +264,7 @@ export default class Generator {
if ( templateProperties.computed ) {
const dependencies = new Map();
templateProperties.computed.properties.forEach( prop => {
templateProperties.computed.value.properties.forEach( prop => {
const key = prop.key.name;
const value = prop.value;
@ -286,7 +286,7 @@ export default class Generator {
computations.push({ key, deps });
}
templateProperties.computed.properties.forEach( prop => visit( prop.key.name ) );
templateProperties.computed.value.properties.forEach( prop => visit( prop.key.name ) );
}
}

@ -156,6 +156,19 @@ export default function dom ( parsed, source, options, names ) {
const { computations, templateProperties } = generator.parseJs();
// Remove these after version 2
if ( templateProperties.onrender ) {
const { key } = templateProperties.onrender;
generator.code.overwrite( key.start, key.end, 'oncreate', true );
templateProperties.oncreate = templateProperties.onrender;
}
if ( templateProperties.onteardown ) {
const { key } = templateProperties.onteardown;
generator.code.overwrite( key.start, key.end, 'ondestroy', true );
templateProperties.ondestroy = templateProperties.onteardown;
}
generator.imports.forEach( node => {
node.specifiers.forEach( specifier => {
generator.importedNames[ specifier.local.name ] = true;
@ -164,7 +177,7 @@ export default function dom ( parsed, source, options, names ) {
let namespace = null;
if ( templateProperties.namespace ) {
const ns = templateProperties.namespace.value;
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
@ -281,12 +294,12 @@ export default function dom ( parsed, source, options, names ) {
builders._set.addBlock( statement );
}
if ( templateProperties.onrender ) {
if ( templateProperties.oncreate ) {
builders.init.addBlock( deindent`
if ( options._root ) {
options._root._renderHooks.push({ fn: template.onrender, context: this });
options._root._renderHooks.push({ fn: template.oncreate, context: this });
} else {
template.onrender.call( this );
template.oncreate.call( this );
}
` );
}
@ -342,13 +355,14 @@ export default function dom ( parsed, source, options, names ) {
${name}.prototype._flush = ${shared._flush};
` );
// TODO deprecate component.teardown()
builders.main.addBlock( deindent`
${name}.prototype._set = function _set ( newState ) {
${builders._set}
};
${name}.prototype.teardown = function teardown ( detach ) {
this.fire( 'teardown' );${templateProperties.onteardown ? `\ntemplate.onteardown.call( this );` : ``}
${name}.prototype.teardown = ${name}.prototype.destroy = function destroy ( detach ) {
this.fire( 'teardown' );${templateProperties.ondestroy ? `\ntemplate.ondestroy.call( this );` : ``}
this._fragment.teardown( detach !== false );
this._fragment = null;

@ -137,7 +137,7 @@ export default {
` );
}
generator.current.builders.teardown.addLine( `${name}.teardown( ${isToplevel ? 'detach' : 'false'} );` );
generator.current.builders.teardown.addLine( `${name}.destroy( ${isToplevel ? 'detach' : 'false'} );` );
generator.current.builders.init.addBlock( local.init );
if ( !local.update.isEmpty() ) generator.current.builders.update.addBlock( local.update );

@ -114,7 +114,7 @@ export default function ssr ( parsed, source, options, names ) {
}
` );
templateProperties.components.properties.forEach( prop => {
templateProperties.components.value.properties.forEach( prop => {
builders.renderCss.addLine( `addComponent( template.components.${prop.key.name} );` );
});
}

@ -36,8 +36,6 @@ export default function validate ( parsed, source, { onerror, onwarn, name, file
});
},
templateProperties: {},
names: [],
namespace: null

@ -23,10 +23,21 @@ export default function validateJs ( validator, js ) {
checkForComputedKeys( validator, node.declaration.properties );
checkForDupes( validator, node.declaration.properties );
const templateProperties = {};
node.declaration.properties.forEach( prop => {
validator.templateProperties[ prop.key.name ] = prop;
templateProperties[ prop.key.name ] = prop;
});
// Remove these checks in version 2
if ( templateProperties.oncreate && templateProperties.onrender ) {
validator.error( 'Cannot have both oncreate and onrender', templateProperties.onrender.start );
}
if ( templateProperties.ondestroy && templateProperties.onteardown ) {
validator.error( 'Cannot have both ondestroy and onteardown', templateProperties.onteardown.start );
}
// ensure all exported props are valid
node.declaration.properties.forEach( prop => {
const propValidator = propValidators[ prop.key.name ];
@ -45,8 +56,8 @@ export default function validateJs ( validator, js ) {
}
});
if ( validator.templateProperties.namespace ) {
const ns = validator.templateProperties.namespace.value.value;
if ( templateProperties.namespace ) {
const ns = templateProperties.namespace.value.value;
validator.namespace = namespaces[ ns ] || ns;
}

@ -1,5 +1,7 @@
import data from './data.js';
import computed from './computed.js';
import oncreate from './oncreate.js';
import ondestroy from './ondestroy.js';
import onrender from './onrender.js';
import onteardown from './onteardown.js';
import helpers from './helpers.js';
@ -11,6 +13,8 @@ import namespace from './namespace.js';
export default {
data,
computed,
oncreate,
ondestroy,
onrender,
onteardown,
helpers,

@ -0,0 +1,9 @@
import usesThisOrArguments from '../utils/usesThisOrArguments.js';
export default function oncreate ( validator, prop ) {
if ( prop.value.type === 'ArrowFunctionExpression' ) {
if ( usesThisOrArguments( prop.value.body ) ) {
validator.error( `'oncreate' should be a function expression, not an arrow function expression`, prop.start );
}
}
}

@ -0,0 +1,9 @@
import usesThisOrArguments from '../utils/usesThisOrArguments.js';
export default function ondestroy ( validator, prop ) {
if ( prop.value.type === 'ArrowFunctionExpression' ) {
if ( usesThisOrArguments( prop.value.body ) ) {
validator.error( `'ondestroy' should be a function expression, not an arrow function expression`, prop.start );
}
}
}

@ -1,9 +1,6 @@
import usesThisOrArguments from '../utils/usesThisOrArguments.js';
import oncreate from './oncreate.js';
export default function onrender ( validator, prop ) {
if ( prop.value.type === 'ArrowFunctionExpression' ) {
if ( usesThisOrArguments( prop.value.body ) ) {
validator.error( `'onrender' should be a function expression, not an arrow function expression`, prop.start );
}
}
validator.warn( `'onrender' has been deprecated in favour of 'oncreate', and will cause an error in Svelte 2.x`, prop.start );
oncreate( validator, prop );
}

@ -1,9 +1,6 @@
import usesThisOrArguments from '../utils/usesThisOrArguments.js';
import ondestroy from './ondestroy.js';
export default function onteardown ( validator, prop ) {
if ( prop.value.type === 'ArrowFunctionExpression' ) {
if ( usesThisOrArguments( prop.value.body ) ) {
validator.error( `'onteardown' should be a function expression, not an arrow function expression`, prop.start );
}
}
validator.warn( `'onteardown' has been deprecated in favour of 'ondestroy', and will cause an error in Svelte 2.x`, prop.start );
ondestroy( validator, prop );
}

@ -17,7 +17,7 @@ function testAmd ( code, expectedId, dependencies, html ) {
assert.htmlEqual( main.innerHTML, html );
component.teardown();
component.destroy();
}
define.amd = true;
@ -44,7 +44,7 @@ function testCjs ( code, dependencyById, html ) {
assert.htmlEqual( main.innerHTML, html );
component.teardown();
component.destroy();
});
}
@ -59,7 +59,7 @@ function testIife ( code, name, globals, html ) {
assert.htmlEqual( main.innerHTML, html );
component.teardown();
component.destroy();
});
}

@ -98,7 +98,7 @@ describe( 'generate', () => {
if ( config.test ) {
config.test( assert, component, target, window );
} else {
component.teardown();
component.destroy();
assert.equal( target.innerHTML, '' );
}
})

@ -24,6 +24,6 @@ export default {
assert.equal( select.value, 'b' );
assert.ok( options[1].selected );
component.teardown();
component.destroy();
}
};

@ -17,6 +17,6 @@ export default {
<p>blah, bar, baz</p>
` );
component.teardown();
component.destroy();
}
};

@ -17,6 +17,6 @@ export default {
<p>blah, bar, baz</p>
` );
component.teardown();
component.destroy();
}
};

@ -22,6 +22,6 @@ export default {
buttons[2].dispatchEvent( event );
assert.deepEqual( clicks, [ 'a', 'c' ]);
component.teardown();
component.destroy();
}
};

@ -2,7 +2,7 @@
<script>
export default {
onrender () {
oncreate () {
this.isWidget = true;
}
};

@ -7,6 +7,6 @@ export default {
component.set({ arriving: false });
assert.htmlEqual( target.innerHTML, `<div><p class='widget'>Goodbye</p></div>` );
component.teardown();
component.destroy();
}
};

@ -4,6 +4,6 @@ export default {
test ( assert, component, target ) {
component.set({ a: 2 });
assert.equal( target.innerHTML, '<p>4</p>' );
component.teardown();
component.destroy();
}
};

@ -5,6 +5,6 @@ export default {
assert.equal( component.get( 'c' ), 5 );
assert.equal( component.get( 'cSquared' ), 25 );
assert.equal( target.innerHTML, '<p>3 + 2 = 5</p>\n<p>5 * 5 = 25</p>' );
component.teardown();
component.destroy();
}
};

@ -3,6 +3,6 @@ export default {
test ( assert, component ) {
assert.equal( component.get( 'foo' ), 'got' );
component.teardown();
component.destroy();
}
};

@ -22,6 +22,6 @@ export default {
assert.equal( count, 1 );
assert.equal( number, 42 );
component.teardown();
component.destroy();
}
};

@ -23,6 +23,6 @@ export default {
assert.ok( !target.contains( p1 ), 'first <p> element should be removed' );
assert.equal( p2, p3, 'second <p> element should be retained' );
component.teardown();
component.destroy();
}
};

@ -12,6 +12,6 @@ export default {
component.fire( 'foo', expected );
assert.equal( count, 1 );
component.teardown();
component.destroy();
}
};

@ -7,7 +7,7 @@ export default {
count += 1;
});
component.teardown();
component.destroy();
assert.equal( count, 1 );
}
};

@ -9,7 +9,7 @@ export default {
component.set({ name: 'everybody' });
assert.htmlEqual( target.innerHTML, '<h1>Hello everybody!</h1>' );
component.teardown();
component.destroy();
assert.htmlEqual( target.innerHTML, '' );
}
};

@ -18,6 +18,6 @@ export default {
before-else-after
` );
component.teardown();
component.destroy();
}
};

@ -18,6 +18,6 @@ export default {
<p>x is between 5 and 10</p>
` );
component.teardown();
component.destroy();
}
};

@ -1,7 +1,7 @@
export default {
test ( assert, component ) {
assert.deepEqual( component.events, [ 'render' ]);
component.teardown();
component.destroy();
assert.deepEqual( component.events, [ 'render', 'teardown' ]);
}
};

@ -2,11 +2,11 @@
<script>
export default {
onrender () {
oncreate () {
this.events = [ 'render' ];
},
onteardown () {
ondestroy () {
this.events.push( 'teardown' );
}
};

@ -7,7 +7,7 @@
foo: 'XX'
};
},
onrender () {
oncreate () {
this.observe( 'item', item => {
this.set({ foo: item });
});

@ -12,6 +12,6 @@ export default {
<span>1</span><span>2</span><span>3</span><span>4</span><span>5</span>
` );
component.teardown();
component.destroy();
}
};

@ -2,7 +2,7 @@
<script>
export default {
onrender () {
oncreate () {
this.set({ inDocument: document.contains( this.refs.x ) })
}
};

@ -1,5 +1,5 @@
export default {
'skip-ssr': true, // uses onrender
'skip-ssr': true, // uses oncreate
html: `<div><p>true</p>\n<p>true</p></div>`
};

@ -2,7 +2,7 @@
<script>
export default {
onrender () {
oncreate () {
this.set({ inDocument: document.contains( this.refs.x ) })
}
};

@ -1,5 +1,5 @@
export default {
'skip-ssr': true, // uses onrender
'skip-ssr': true, // uses oncreate
html: `<div><p>true</p></div>`,
@ -7,6 +7,6 @@ export default {
component.set({ foo: true });
assert.htmlEqual( target.innerHTML, `<div><p>true</p>\n<p>true</p></div>` );
component.teardown();
component.destroy();
}
};

@ -16,6 +16,6 @@ export default {
assert.equal( target.innerHTML, `<div>${ns}<p>does not change</p>${ns}</div>` );
assert.strictEqual( target.querySelector( 'p' ), p );
component.teardown();
component.destroy();
}
};

@ -14,7 +14,7 @@ export default {
assert.equal( target.innerHTML, `before${ns}${ns}after` );
component.set({ raw: 'how about <strong>unclosed elements?' });
assert.equal( target.innerHTML, `before${ns}how about <strong>unclosed elements?</strong>${ns}after` );
component.teardown();
component.destroy();
assert.equal( target.innerHTML, '' );
}
};

@ -25,7 +25,7 @@ export default {
assert.equal( target.querySelector( 'select' ).value, 'a' );
component.teardown();
component.destroy();
assert.htmlEqual( target.innerHTML, '' );
}
};

@ -2,7 +2,7 @@
<script>
export default {
onrender () {
oncreate () {
this.observe( 'foo', foo => {
const bar = this.get( 'bar' );
if ( foo.x !== bar.x ) throw new Error( 'mismatch' );

@ -3,6 +3,6 @@ export default {
test ( assert, component ) {
component.set({ foo: { x: 2 } });
component.teardown();
component.destroy();
}
};

@ -11,7 +11,7 @@
}
},
onrender () {
oncreate () {
this.observe( 'foo', foo => {
this.set({ bar: foo });
});

@ -1,5 +1,5 @@
export default {
'skip-ssr': true, // uses onrender
'skip-ssr': true, // uses oncreate
html: `
<p>1</p>

@ -7,7 +7,7 @@
foo: 1
}),
onrender () {
oncreate () {
this.observe( 'foo', foo => {
this.set({ bar: foo * 2 });
});

@ -1,5 +1,5 @@
export default {
'skip-ssr': true, // uses onrender
'skip-ssr': true, // uses oncreate
html: '<p>2</p>'
};

@ -6,7 +6,7 @@
foo: 1
}),
onrender () {
oncreate () {
this.set({ foo: 2 });
}
};

@ -10,6 +10,6 @@ export default {
test ( assert, component, target ) {
const circle = target.querySelector( 'circle' );
assert.equal( circle.getAttribute( 'class' ), 'red' );
component.teardown();
component.destroy();
}
};

@ -9,6 +9,6 @@ export default {
assert.equal( svg.namespaceURI, 'http://www.w3.org/2000/svg' );
assert.equal( svg.getAttribute( 'class' ), 'foo' );
component.teardown();
component.destroy();
}
};

@ -10,6 +10,6 @@ export default {
assert.equal( circles[0].namespaceURI, 'http://www.w3.org/2000/svg' );
assert.equal( circles[1].namespaceURI, 'http://www.w3.org/2000/svg' );
assert.equal( circles[2].namespaceURI, 'http://www.w3.org/2000/svg' );
component.teardown();
component.destroy();
}
};

@ -14,6 +14,6 @@ export default {
assert.equal( href.namespaceURI, 'http://www.w3.org/1999/xlink' );
component.teardown();
component.destroy();
}
};

@ -2,7 +2,7 @@
<script>
export default {
onrender () {
oncreate () {
console.log( 42 );
}
}

@ -0,0 +1,5 @@
<script>
export default {
oncreate: () => console.log( 'rendering' )
};
</script>

@ -1,5 +1,5 @@
[{
"message": "'onrender' should be a function expression, not an arrow function expression",
"message": "'oncreate' should be a function expression, not an arrow function expression",
"pos": 29,
"loc": {
"line": 3,

@ -1,6 +1,6 @@
<script>
export default {
onrender: () => {
oncreate: () => {
this.set({ a: 1 });
}
};

@ -0,0 +1,5 @@
<script>
export default {
ondestroy: () => console.log( 'tearing down' )
};
</script>

@ -0,0 +1,8 @@
[{
"message": "'ondestroy' should be a function expression, not an arrow function expression",
"pos": 29,
"loc": {
"line": 3,
"column": 2
}
}]

@ -1,6 +1,6 @@
<script>
export default {
onteardown: () => {
ondestroy: () => {
this.set({ a: 1 });
}
};

@ -1,5 +0,0 @@
<script>
export default {
onrender: () => console.log( 'rendering' )
};
</script>

@ -1,5 +0,0 @@
<script>
export default {
onteardown: () => console.log( 'tearing down' )
};
</script>

@ -1,8 +0,0 @@
[{
"message": "'onteardown' should be a function expression, not an arrow function expression",
"pos": 29,
"loc": {
"line": 3,
"column": 2
}
}]
Loading…
Cancel
Save