From 146f6455025f1a5989f87320f1423c8095762fa6 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sun, 24 Dec 2017 13:38:57 -0500 Subject: [PATCH] detect unused/misplaced components - closes #1039 --- src/generators/dom/index.ts | 10 ++++---- src/generators/server-side-rendering/index.ts | 2 +- src/validate/html/validateElement.ts | 4 ++++ src/validate/index.ts | 24 +++++++++++++++++++ .../Bar.html | 1 + .../Foo.html | 1 + .../_config.js | 14 +++++++++++ .../main.html | 13 ++++++++++ .../samples/component-unused/input.html | 11 +++++++++ .../samples/component-unused/warnings.json | 18 ++++++++++++++ .../input.html | 2 +- 11 files changed, 94 insertions(+), 6 deletions(-) create mode 100644 test/runtime/samples/dev-warning-dynamic-components-misplaced/Bar.html create mode 100644 test/runtime/samples/dev-warning-dynamic-components-misplaced/Foo.html create mode 100644 test/runtime/samples/dev-warning-dynamic-components-misplaced/_config.js create mode 100644 test/runtime/samples/dev-warning-dynamic-components-misplaced/main.html create mode 100644 test/validator/samples/component-unused/input.html create mode 100644 test/validator/samples/component-unused/warnings.json diff --git a/src/generators/dom/index.ts b/src/generators/dom/index.ts index b8fb36f80104..487006ad5310 100644 --- a/src/generators/dom/index.ts +++ b/src/generators/dom/index.ts @@ -212,10 +212,12 @@ export default function dom( ${generator.metaBindings} ${computations.length && `this._recompute({ ${Array.from(computationDeps).map(dep => `${dep}: 1`).join(', ')} }, this._state);`} ${options.dev && - Array.from(generator.expectedProperties).map( - prop => - `if (!('${prop}' in this._state)) console.warn("${debugName} was created without expected data property '${prop}'");` - )} + Array.from(generator.expectedProperties).map(prop => { + const message = generator.components.has(prop) ? + `${debugName} expected to find '${prop}' in \`data\`, but found it in \`components\` instead` : + `${debugName} was created without expected data property '${prop}'`; + return `if (!('${prop}' in this._state)) console.warn("${message}");` + })} ${generator.bindingGroups.length && `this._bindingGroups = [${Array(generator.bindingGroups.length).fill('[]').join(', ')}];`} diff --git a/src/generators/server-side-rendering/index.ts b/src/generators/server-side-rendering/index.ts index 85118ef5fb48..37bf859da7c7 100644 --- a/src/generators/server-side-rendering/index.ts +++ b/src/generators/server-side-rendering/index.ts @@ -232,7 +232,7 @@ export default function ssr( ${ /__missingComponent/.test(generator.renderCode) && deindent` var __missingComponent = { - render: () => '' + _render: () => '' }; ` } diff --git a/src/validate/html/validateElement.ts b/src/validate/html/validateElement.ts index e29066be0187..7c1af7d37110 100644 --- a/src/validate/html/validateElement.ts +++ b/src/validate/html/validateElement.ts @@ -16,6 +16,10 @@ export default function validateElement( const isComponent = node.name === ':Self' || node.name === ':Component' || validator.components.has(node.name); + if (isComponent) { + validator.used.components.add(node.name); + } + if (!isComponent && /^[A-Z]/.test(node.name[0])) { // TODO upgrade to validator.error in v2 validator.warn(`${node.name} component is not defined`, node.start); diff --git a/src/validate/index.ts b/src/validate/index.ts index 5ed4d58bd9f6..e7bc9c5262cf 100644 --- a/src/validate/index.ts +++ b/src/validate/index.ts @@ -35,6 +35,10 @@ export class Validator { transitions: Map; slots: Set; + used: { + components: Set + }; + constructor(parsed: Parsed, source: string, options: CompileOptions) { this.source = source; this.filename = options.filename; @@ -50,6 +54,10 @@ export class Validator { this.helpers = new Map(); this.transitions = new Map(); this.slots = new Set(); + + this.used = { + components: new Set() + }; } error(message: string, pos: number) { @@ -114,6 +122,22 @@ export default function validate( if (parsed.html) { validateHtml(validator, parsed.html); } + + // need to do a second pass of the JS, now that we've analysed the markup + if (parsed.js && validator.defaultExport) { + const components = validator.defaultExport.declaration.properties.find(prop => prop.key.name === 'components'); + if (components) { + components.value.properties.forEach(prop => { + const { name } = prop.key; + if (!validator.used.components.has(name)) { + validator.warn( + `The ${name} component is unused`, + prop.start + ); + } + }); + } + } } catch (err) { if (onerror) { onerror(err); diff --git a/test/runtime/samples/dev-warning-dynamic-components-misplaced/Bar.html b/test/runtime/samples/dev-warning-dynamic-components-misplaced/Bar.html new file mode 100644 index 000000000000..d9d3a9a899e4 --- /dev/null +++ b/test/runtime/samples/dev-warning-dynamic-components-misplaced/Bar.html @@ -0,0 +1 @@ +Bar \ No newline at end of file diff --git a/test/runtime/samples/dev-warning-dynamic-components-misplaced/Foo.html b/test/runtime/samples/dev-warning-dynamic-components-misplaced/Foo.html new file mode 100644 index 000000000000..9f26b637f017 --- /dev/null +++ b/test/runtime/samples/dev-warning-dynamic-components-misplaced/Foo.html @@ -0,0 +1 @@ +Foo \ No newline at end of file diff --git a/test/runtime/samples/dev-warning-dynamic-components-misplaced/_config.js b/test/runtime/samples/dev-warning-dynamic-components-misplaced/_config.js new file mode 100644 index 000000000000..28a38333e887 --- /dev/null +++ b/test/runtime/samples/dev-warning-dynamic-components-misplaced/_config.js @@ -0,0 +1,14 @@ +export default { + dev: true, + + data: { + x: true + }, + + html: '', + + warnings: [ + ` expected to find 'Foo' in \`data\`, but found it in \`components\` instead`, + ` expected to find 'Bar' in \`data\`, but found it in \`components\` instead` + ] +}; \ No newline at end of file diff --git a/test/runtime/samples/dev-warning-dynamic-components-misplaced/main.html b/test/runtime/samples/dev-warning-dynamic-components-misplaced/main.html new file mode 100644 index 000000000000..37392f27e6fd --- /dev/null +++ b/test/runtime/samples/dev-warning-dynamic-components-misplaced/main.html @@ -0,0 +1,13 @@ +<:Component {x ? Foo : Bar}/> + + \ No newline at end of file diff --git a/test/validator/samples/component-unused/input.html b/test/validator/samples/component-unused/input.html new file mode 100644 index 000000000000..db1ba9f4c0fb --- /dev/null +++ b/test/validator/samples/component-unused/input.html @@ -0,0 +1,11 @@ + \ No newline at end of file diff --git a/test/validator/samples/component-unused/warnings.json b/test/validator/samples/component-unused/warnings.json new file mode 100644 index 000000000000..4692da33aa5a --- /dev/null +++ b/test/validator/samples/component-unused/warnings.json @@ -0,0 +1,18 @@ +[ + { + "message": "The Foo component is unused", + "loc": { + "line": 7, + "column": 3 + }, + "pos": 109 + }, + { + "message": "The Bar component is unused", + "loc": { + "line": 8, + "column": 3 + }, + "pos": 117 + } +] diff --git a/test/validator/samples/properties-components-should-be-capitalised/input.html b/test/validator/samples/properties-components-should-be-capitalised/input.html index 3052ff968ef7..627a614cff58 100644 --- a/test/validator/samples/properties-components-should-be-capitalised/input.html +++ b/test/validator/samples/properties-components-should-be-capitalised/input.html @@ -1,4 +1,4 @@ -
+