From 8b38b2b802d7770a856757e4c4648b93a8928d5b Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 10 Apr 2017 17:47:47 -0400 Subject: [PATCH] throw if user sets read-only properties in dev mode --- src/generators/dom/index.js | 17 +++++++++++++++++ .../dom/visitors/Element/meta/Window.js | 17 +++++++++++++++++ .../dev-warning-readonly-computed/_config.js | 12 ++++++++++++ .../dev-warning-readonly-computed/main.html | 7 +++++++ .../_config.js | 12 ++++++++++++ .../main.html | 1 + 6 files changed, 66 insertions(+) create mode 100644 test/runtime/samples/dev-warning-readonly-computed/_config.js create mode 100644 test/runtime/samples/dev-warning-readonly-computed/main.html create mode 100644 test/runtime/samples/dev-warning-readonly-window-binding/_config.js create mode 100644 test/runtime/samples/dev-warning-readonly-window-binding/main.html diff --git a/src/generators/dom/index.js b/src/generators/dom/index.js index dbb054e7c6..50d82ba31b 100644 --- a/src/generators/dom/index.js +++ b/src/generators/dom/index.js @@ -11,6 +11,8 @@ class DomGenerator extends Generator { this.blocks = []; this.uses = new Set(); + this.readonly = new Set(); + // initial values for e.g. window.innerWidth, if there's a <:Window> meta tag this.builders = { metaBindings: new CodeBuilder() @@ -94,6 +96,13 @@ export default function dom ( parsed, source, options ) { const differs = generator.helper( 'differs' ); 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 ); + 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( ', ' )} ); @@ -106,7 +115,15 @@ export default function dom ( parsed, source, options ) { ${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 )` ); } diff --git a/src/generators/dom/visitors/Element/meta/Window.js b/src/generators/dom/visitors/Element/meta/Window.js index 8030d68090..5bf02b2774 100644 --- a/src/generators/dom/visitors/Element/meta/Window.js +++ b/src/generators/dom/visitors/Element/meta/Window.js @@ -12,6 +12,14 @@ const associatedEvents = { scrollY: 'scroll' }; +const readonly = new Set([ + 'innerWidth', + 'innerHeight', + 'outerWidth', + 'outerHeight', + 'online' +]); + export default function visitWindow ( generator, block, node ) { const events = {}; 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}'` ); } + // 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; // 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;` ); } + if ( generator.options.dev ) handlerBody.addLine( `component._updatingReadonlyProperty = true;` ); + handlerBody.addBlock( deindent` component.set({ ${props} }); ` ); + if ( generator.options.dev ) handlerBody.addLine( `component._updatingReadonlyProperty = false;` ); + if ( event === 'scroll' ) { handlerBody.addLine( `${lock} = false;` ); } diff --git a/test/runtime/samples/dev-warning-readonly-computed/_config.js b/test/runtime/samples/dev-warning-readonly-computed/_config.js new file mode 100644 index 0000000000..afa3a65667 --- /dev/null +++ b/test/runtime/samples/dev-warning-readonly-computed/_config.js @@ -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'` ); + } + } +}; \ No newline at end of file diff --git a/test/runtime/samples/dev-warning-readonly-computed/main.html b/test/runtime/samples/dev-warning-readonly-computed/main.html new file mode 100644 index 0000000000..d6bb5eb81c --- /dev/null +++ b/test/runtime/samples/dev-warning-readonly-computed/main.html @@ -0,0 +1,7 @@ + \ No newline at end of file diff --git a/test/runtime/samples/dev-warning-readonly-window-binding/_config.js b/test/runtime/samples/dev-warning-readonly-window-binding/_config.js new file mode 100644 index 0000000000..81d48e1619 --- /dev/null +++ b/test/runtime/samples/dev-warning-readonly-window-binding/_config.js @@ -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'` ); + } + } +}; \ No newline at end of file diff --git a/test/runtime/samples/dev-warning-readonly-window-binding/main.html b/test/runtime/samples/dev-warning-readonly-window-binding/main.html new file mode 100644 index 0000000000..635ae75e9c --- /dev/null +++ b/test/runtime/samples/dev-warning-readonly-window-binding/main.html @@ -0,0 +1 @@ +<:Window bind:innerWidth='width'/> \ No newline at end of file