From 43bb971c9e986fb79be70b92f4a525b6415b0202 Mon Sep 17 00:00:00 2001 From: Nico Rehwaldt Date: Thu, 8 Dec 2016 17:12:18 +0100 Subject: [PATCH 1/2] Log errors with correct context Allow webpack to print verbose error logs, including the error context (in case of parse errors). --- index.js | 11 +++++------ test/fixtures/bad.html | 4 ++-- test/loader.spec.js | 18 ++++++++++++++---- 3 files changed, 21 insertions(+), 12 deletions(-) diff --git a/index.js b/index.js index 10872dda..3fb6d251 100644 --- a/index.js +++ b/index.js @@ -9,22 +9,21 @@ module.exports = function(source, map) { try { let { code, map } = compile(source, { - // name: CamelCase component name filename: filename, format: query.format || 'es', name: query.name, onerror: (err) => { - console.log(err.message); - this.emitError(err.message); + this.emitError(err); }, onwarn: (warn) => { - console.log(warn.message); - this.emitWarn(warn.message); + this.emitWarn(warn); } }); this.callback(null, code, map); } catch (err) { - this.callback(err); + // wrap error to provide correct + // context when logging to console + this.callback(new Error(err.toString())); } }; diff --git a/test/fixtures/bad.html b/test/fixtures/bad.html index 4481171c..eaa6d49a 100644 --- a/test/fixtures/bad.html +++ b/test/fixtures/bad.html @@ -1,2 +1,2 @@ -

Count: {{count}}

- \ No newline at end of file +

Count: {{{count}}

+ \ No newline at end of file diff --git a/test/loader.spec.js b/test/loader.spec.js index fc0b0c2e..9a42fa65 100644 --- a/test/loader.spec.js +++ b/test/loader.spec.js @@ -19,17 +19,19 @@ describe('loader', function() { return function() { + const fileContents = readFile(fileName); + const cacheableSpy = spy(function() { }); const callbackSpy = spy(callback); - const fileContents = readFile(fileName); - loader.call({ cacheable: cacheableSpy, callback: callbackSpy, + emitWarn: function(warning) {}, + emitError: function(e) {}, filename: fileName, - query: query, + query, }, fileContents, null); expect(callbackSpy).to.have.been.called; @@ -49,9 +51,17 @@ describe('loader', function() { it('should compile bad', - testLoader('test/fixtures/bad.html', function(err, code, map) { + testLoader('test/fixtures/bad.html', function(err, code, map, context) { + expect(err).to.exist; + expect(err.message).to.eql( + 'Expected }}} (1:18)\n' + + '1:

Count: {{{count}}

\n' + + ' ^\n' + + '2: ' + ); + expect(code).not.to.exist; expect(map).not.to.exist; }) From 2d6b988abc1b25fb7fb9767e7733fa8a53ab002c Mon Sep 17 00:00:00 2001 From: Nico Rehwaldt Date: Thu, 8 Dec 2016 17:41:47 +0100 Subject: [PATCH 2/2] Fix loader: throw validation errors * add test verifying that we properly throw validation errors * restructure test suite --- index.js | 8 +- test/fixtures/export-error.html | 7 + test/fixtures/{bad.html => parse-error.html} | 0 test/fixtures/validation-error.html | 9 ++ test/loader.spec.js | 146 +++++++++++++------ 5 files changed, 116 insertions(+), 54 deletions(-) create mode 100644 test/fixtures/export-error.html rename test/fixtures/{bad.html => parse-error.html} (100%) create mode 100644 test/fixtures/validation-error.html diff --git a/index.js b/index.js index 3fb6d251..e5af8f03 100644 --- a/index.js +++ b/index.js @@ -11,13 +11,7 @@ module.exports = function(source, map) { let { code, map } = compile(source, { filename: filename, format: query.format || 'es', - name: query.name, - onerror: (err) => { - this.emitError(err); - }, - onwarn: (warn) => { - this.emitWarn(warn); - } + name: query.name }); this.callback(null, code, map); diff --git a/test/fixtures/export-error.html b/test/fixtures/export-error.html new file mode 100644 index 00000000..52d97ad8 --- /dev/null +++ b/test/fixtures/export-error.html @@ -0,0 +1,7 @@ +

Count: {{count}}

+ + \ No newline at end of file diff --git a/test/fixtures/bad.html b/test/fixtures/parse-error.html similarity index 100% rename from test/fixtures/bad.html rename to test/fixtures/parse-error.html diff --git a/test/fixtures/validation-error.html b/test/fixtures/validation-error.html new file mode 100644 index 00000000..e152e795 --- /dev/null +++ b/test/fixtures/validation-error.html @@ -0,0 +1,9 @@ +

Count: {{count}}

+ + \ No newline at end of file diff --git a/test/loader.spec.js b/test/loader.spec.js index 9a42fa65..4b34f42c 100644 --- a/test/loader.spec.js +++ b/test/loader.spec.js @@ -28,8 +28,6 @@ describe('loader', function() { loader.call({ cacheable: cacheableSpy, callback: callbackSpy, - emitWarn: function(warning) {}, - emitError: function(e) {}, filename: fileName, query, }, fileContents, null); @@ -40,7 +38,7 @@ describe('loader', function() { } - it('should compile good', + it('should compile', testLoader('test/fixtures/good.html', function(err, code, map) { expect(err).not.to.exist; @@ -50,65 +48,119 @@ describe('loader', function() { ); - it('should compile bad', - testLoader('test/fixtures/bad.html', function(err, code, map, context) { + describe('error handling', function() { - expect(err).to.exist; + it('should handle parse error', + testLoader('test/fixtures/parse-error.html', function(err, code, map, context) { - expect(err.message).to.eql( - 'Expected }}} (1:18)\n' + - '1:

Count: {{{count}}

\n' + - ' ^\n' + - '2: ' - ); + expect(err).to.exist; - expect(code).not.to.exist; - expect(map).not.to.exist; - }) - ); + expect(err.message).to.eql( + 'Expected }}} (1:18)\n' + + '1:

Count: {{{count}}

\n' + + ' ^\n' + + '2: ' + ); + expect(code).not.to.exist; + expect(map).not.to.exist; + }) + ); - it('should compile with import / ES2015 features', - testLoader('test/fixtures/es2015.html', function(err, code, map) { - expect(err).not.to.exist; - expect(code).to.exist; - expect(map).to.exist; + it('should handle wrong export', + testLoader('test/fixtures/export-error.html', function(err, code, map, context) { - // es2015 statements remain - expect(code).to.contain('import { hello } from \'./utils\';'); - expect(code).to.contain('data() {'); - }) - ); + expect(err).to.exist; + expect(err.message).to.eql( + 'Unexpected token (5:7)\n' + + '3: ' + ); - it('should compile Component with with nesting', - testLoader('test/fixtures/parent.html', function(err, code, map) { - expect(err).not.to.exist; + expect(code).not.to.exist; + expect(map).not.to.exist; + }) + ); - // es2015 statements remain - expect(code).to.contain('import Nested from \'./nested\';'); - expect(code).to.exist; - expect(map).to.exist; - }) - ); + it('should validation error', + testLoader('test/fixtures/validation-error.html', function(err, code, map, context) { + expect(err).to.exist; - it('should compile Component to CJS if requested', - testLoader('test/fixtures/good.html', function(err, code, map) { - expect(err).not.to.exist; - expect(code).to.contain('module.exports = SvelteComponent;'); - }, { format: 'cjs' }) - ); + expect(err.message).to.eql( + 'Computed properties can be function expressions or arrow function expressions (6:11)\n' + + '4: export default {\n' + + '5: computed: {\n' + + '6: foo: \'BAR\'\n' + + ' ^\n' + + '7: }\n' + + '8: };' + ); + expect(code).not.to.exist; + expect(map).not.to.exist; + }) + ); - it('should compile Component to UMD if requested', - testLoader('test/fixtures/good.html', function(err, code, map) { - expect(err).not.to.exist; - expect(code).to.contain('(global.FooComponent = factory());'); - }, { format: 'umd', name: 'FooComponent' }) - ); + }); + + + describe('ES2015 features', function() { + + it('should keep imports / methods', + testLoader('test/fixtures/es2015.html', function(err, code, map) { + expect(err).not.to.exist; + + expect(code).to.exist; + expect(map).to.exist; + + // es2015 statements remain + expect(code).to.contain('import { hello } from \'./utils\';'); + expect(code).to.contain('data() {'); + }) + ); + + + it('should keep nested Component import', + testLoader('test/fixtures/parent.html', function(err, code, map) { + expect(err).not.to.exist; + + // es2015 statements remain + expect(code).to.contain('import Nested from \'./nested\';'); + + expect(code).to.exist; + expect(map).to.exist; + }) + ); + + }); + + + describe('configuration via query', function() { + + it('should configure CommonJS output', + testLoader('test/fixtures/good.html', function(err, code, map) { + expect(err).not.to.exist; + expect(code).to.contain('module.exports = SvelteComponent;'); + }, { format: 'cjs' }) + ); + + + it('should configure named UMD output', + testLoader('test/fixtures/good.html', function(err, code, map) { + expect(err).not.to.exist; + expect(code).to.contain('(global.FooComponent = factory());'); + }, { format: 'umd', name: 'FooComponent' }) + ); + + }); });