Merge pull request #463 from sveltejs/readonly-dev-warning

In dev mode, throw if user sets a read-only property
pull/471/head
Rich Harris 8 years ago committed by GitHub
commit 262f4aaf49

@ -11,6 +11,8 @@ class DomGenerator extends Generator {
this.blocks = []; this.blocks = [];
this.uses = new Set(); this.uses = new Set();
this.readonly = new Set();
// initial values for e.g. window.innerWidth, if there's a <:Window> meta tag // initial values for e.g. window.innerWidth, if there's a <:Window> meta tag
this.builders = { this.builders = {
metaBindings: new CodeBuilder() metaBindings: new CodeBuilder()
@ -94,6 +96,13 @@ export default function dom ( parsed, source, options ) {
const differs = generator.helper( 'differs' ); const differs = generator.helper( 'differs' );
computations.forEach( ({ key, deps }) => { computations.forEach( ({ key, deps }) => {
if ( generator.readonly.has( key ) ) {
// <:Window> bindings
throw new Error( `Cannot have a computed value '${key}' that clashes with a read-only property` );
}
generator.readonly.add( key );
const condition = `isInitial || ${deps.map( dep => `( '${dep}' in newState && ${differs}( state.${dep}, oldState.${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( ', ' )} );`; const statement = `state.${key} = newState.${key} = ${generator.alias( 'template' )}.computed.${key}( ${deps.map( dep => `state.${dep}` ).join( ', ' )} );`;
@ -105,7 +114,15 @@ export default function dom ( parsed, source, options ) {
${builder} ${builder}
} }
` ); ` );
}
if ( options.dev ) {
Array.from( generator.readonly ).forEach( prop => {
builders._set.addLine( `if ( '${prop}' in newState && !this._updatingReadonlyProperty ) throw new Error( "Cannot set read-only property '${prop}'" );` );
});
}
if ( computations.length ) {
builders._set.addLine( `${generator.alias( 'recompute' )}( this._state, newState, oldState, false )` ); builders._set.addLine( `${generator.alias( 'recompute' )}( this._state, newState, oldState, false )` );
} }

@ -12,6 +12,14 @@ const associatedEvents = {
scrollY: 'scroll' scrollY: 'scroll'
}; };
const readonly = new Set([
'innerWidth',
'innerHeight',
'outerWidth',
'outerHeight',
'online'
]);
export default function visitWindow ( generator, block, node ) { export default function visitWindow ( generator, block, node ) {
const events = {}; const events = {};
const bindings = {}; const bindings = {};
@ -47,6 +55,11 @@ export default function visitWindow ( generator, block, node ) {
throw new Error( `Bindings on <:Window/> must be to top-level properties, e.g. '${parts.pop()}' rather than '${keypath}'` ); throw new Error( `Bindings on <:Window/> must be to top-level properties, e.g. '${parts.pop()}' rather than '${keypath}'` );
} }
// in dev mode, throw if read-only values are written to
if ( readonly.has( attribute.name ) ) {
generator.readonly.add( attribute.value.name );
}
bindings[ attribute.name ] = attribute.value.name; bindings[ attribute.name ] = attribute.value.name;
// bind:online is a special case, we need to listen for two separate events // bind:online is a special case, we need to listen for two separate events
@ -80,12 +93,16 @@ export default function visitWindow ( generator, block, node ) {
handlerBody.addLine( `${lock} = true;` ); handlerBody.addLine( `${lock} = true;` );
} }
if ( generator.options.dev ) handlerBody.addLine( `component._updatingReadonlyProperty = true;` );
handlerBody.addBlock( deindent` handlerBody.addBlock( deindent`
component.set({ component.set({
${props} ${props}
}); });
` ); ` );
if ( generator.options.dev ) handlerBody.addLine( `component._updatingReadonlyProperty = false;` );
if ( event === 'scroll' ) { if ( event === 'scroll' ) {
handlerBody.addLine( `${lock} = false;` ); handlerBody.addLine( `${lock} = false;` );
} }

@ -0,0 +1,12 @@
export default {
dev: true,
test ( assert, component ) {
try {
component.set({ foo: 1 });
throw new Error( 'Expected an error' );
} catch ( err ) {
assert.equal( err.message, `Cannot set read-only property 'foo'` );
}
}
};

@ -0,0 +1,7 @@
<script>
export default {
computed: {
foo: a => a + 1
}
};
</script>

@ -0,0 +1,12 @@
export default {
dev: true,
test ( assert, component ) {
try {
component.set({ width: 99 });
throw new Error( 'Expected an error' );
} catch ( err ) {
assert.equal( err.message, `Cannot set read-only property 'width'` );
}
}
};
Loading…
Cancel
Save