From 81c5c480e896b079aafe704e99699b65973f9db2 Mon Sep 17 00:00:00 2001 From: Tan Li Hau Date: Sun, 27 Oct 2019 13:42:50 +0800 Subject: [PATCH 1/3] feat: warn unused exports --- src/compiler/compile/Component.ts | 253 ++++++++++++------ .../compile/utils/is_used_as_reference.ts | 33 +++ src/compiler/interfaces.ts | 3 +- .../unreferenced-variables/input.svelte | 21 ++ .../unreferenced-variables/warnings.json | 77 ++++++ .../vars/samples/$$props-logicless/_config.js | 1 + test/vars/samples/$$props/_config.js | 1 + test/vars/samples/actions/_config.js | 2 + test/vars/samples/animations/_config.js | 2 + .../samples/component-namespaced/_config.js | 1 + .../duplicate-non-hoistable/_config.js | 1 + test/vars/samples/duplicate-vars/_config.js | 2 + .../vars/samples/implicit-reactive/_config.js | 2 + test/vars/samples/imports/_config.js | 3 + .../mutated-vs-reassigned-bindings/_config.js | 2 + .../samples/mutated-vs-reassigned/_config.js | 2 + test/vars/samples/props/_config.js | 4 + .../samples/referenced-from-script/_config.js | 160 +++++++++++ .../referenced-from-script/input.svelte | 16 ++ test/vars/samples/store-referenced/_config.js | 2 + .../samples/store-unreferenced/_config.js | 2 + .../samples/template-references/_config.js | 3 + test/vars/samples/transitions/_config.js | 6 + 23 files changed, 511 insertions(+), 88 deletions(-) create mode 100644 src/compiler/compile/utils/is_used_as_reference.ts create mode 100644 test/validator/samples/unreferenced-variables/input.svelte create mode 100644 test/validator/samples/unreferenced-variables/warnings.json create mode 100644 test/vars/samples/referenced-from-script/_config.js create mode 100644 test/vars/samples/referenced-from-script/input.svelte diff --git a/src/compiler/compile/Component.ts b/src/compiler/compile/Component.ts index 3bd7373f7f..cc3bf2e308 100644 --- a/src/compiler/compile/Component.ts +++ b/src/compiler/compile/Component.ts @@ -18,6 +18,7 @@ import { Ast, CompileOptions, Var, Warning } from '../interfaces'; import error from '../utils/error'; import get_code_frame from '../utils/get_code_frame'; import flatten_reference from './utils/flatten_reference'; +import is_used_as_reference from './utils/is_used_as_reference'; import is_reference from 'is-reference'; import TemplateScope from './nodes/shared/TemplateScope'; import fuzzymatch from '../utils/fuzzymatch'; @@ -168,12 +169,13 @@ export default class Component { this.tag = this.name.name; } - this.walk_module_js(); + this.walk_module_js_pre_template(); this.walk_instance_js_pre_template(); this.fragment = new Fragment(this, ast.html); this.name = this.get_unique_name(name); + this.walk_module_js_post_template(); this.walk_instance_js_post_template(); if (!compile_options.customElement) this.stylesheet.reify(); @@ -346,6 +348,7 @@ export default class Component { reassigned: v.reassigned || false, referenced: v.referenced || false, writable: v.writable || false, + referenced_from_script: v.referenced_from_script || false, })), stats: this.stats.render(), }; @@ -447,63 +450,64 @@ export default class Component { }); } - extract_imports(content) { - for (let i = 0; i < content.body.length; i += 1) { - const node = content.body[i]; - - if (node.type === 'ImportDeclaration') { - content.body.splice(i--, 1); - this.imports.push(node); - } - } + extract_imports(node) { + this.imports.push(node); } - extract_exports(content) { - let i = content.body.length; - while (i--) { - const node = content.body[i]; + extract_exports(node) { + if (node.type === 'ExportDefaultDeclaration') { + this.error(node, { + code: `default-export`, + message: `A component cannot have a default export`, + }); + } - if (node.type === 'ExportDefaultDeclaration') { + if (node.type === 'ExportNamedDeclaration') { + if (node.source) { this.error(node, { - code: `default-export`, - message: `A component cannot have a default export`, + code: `not-implemented`, + message: `A component currently cannot have an export ... from`, }); } - - if (node.type === 'ExportNamedDeclaration') { - if (node.source) { - this.error(node, { - code: `not-implemented`, - message: `A component currently cannot have an export ... from`, + if (node.declaration) { + if (node.declaration.type === 'VariableDeclaration') { + node.declaration.declarations.forEach(declarator => { + extract_names(declarator.id).forEach(name => { + const variable = this.var_lookup.get(name); + variable.export_name = name; + if (variable.writable && !(variable.referenced || variable.referenced_from_script)) { + this.warn(declarator, { + code: `unused-export-let`, + message: `${this.name.name} has unused export property '${name}'. If it is for external reference only, please consider using \`export const '${name}'\`` + }); + } + }); }); + } else { + const { name } = node.declaration.id; + + const variable = this.var_lookup.get(name); + variable.export_name = name; } - if (node.declaration) { - if (node.declaration.type === 'VariableDeclaration') { - node.declaration.declarations.forEach(declarator => { - extract_names(declarator.id).forEach(name => { - const variable = this.var_lookup.get(name); - variable.export_name = name; - }); - }); - } else { - const { name } = node.declaration.id; - const variable = this.var_lookup.get(name); - variable.export_name = name; - } + return node.declaration; + } else { + node.specifiers.forEach(specifier => { + const variable = this.var_lookup.get(specifier.local.name); - content.body[i] = node.declaration; - } else { - node.specifiers.forEach(specifier => { - const variable = this.var_lookup.get(specifier.local.name); + if (variable) { + variable.export_name = specifier.exported.name; - if (variable) { - variable.export_name = specifier.exported.name; + if (variable.writable && !(variable.referenced || variable.referenced_from_script)) { + this.warn(specifier, { + code: `unused-export-let`, + message: `${this.name.name} has unused export property '${specifier.exported.name}'. If it is for external reference only, please consider using \`export const '${specifier.exported.name}'\`` + }); } - }); + } + }); - content.body.splice(i, 1); - } + return null; } } } @@ -522,7 +526,7 @@ export default class Component { }); } - walk_module_js() { + walk_module_js_pre_template() { const component = this; const script = this.ast.module; if (!script) return; @@ -573,9 +577,6 @@ export default class Component { }); } }); - - this.extract_imports(script.content); - this.extract_exports(script.content); } walk_instance_js_pre_template() { @@ -657,7 +658,10 @@ export default class Component { this.add_reference(name.slice(1)); const variable = this.var_lookup.get(name.slice(1)); - if (variable) variable.subscribable = true; + if (variable) { + variable.subscribable = true; + variable.referenced_from_script = true; + } } else { this.add_var({ name, @@ -667,46 +671,83 @@ export default class Component { } }); - this.extract_imports(script.content); - this.extract_exports(script.content); - this.track_mutations(); + this.track_references_and_mutations(); + } + + walk_module_js_post_template() { + const script = this.ast.module; + if (!script) return; + + const { body } = script.content; + let i = body.length; + while(--i >= 0) { + const node = body[i]; + if (node.type === 'ImportDeclaration') { + this.extract_imports(node); + body.splice(i, 1); + } + + if (/^Export/.test(node.type)) { + const replacement = this.extract_exports(node); + if (replacement) { + body[i] = replacement; + } else { + body.splice(i, 1); + } + } + } } walk_instance_js_post_template() { const script = this.ast.instance; if (!script) return; - this.warn_on_undefined_store_value_references(); + this.post_template_walk(); + this.hoist_instance_declarations(); this.extract_reactive_declarations(); } - // TODO merge this with other walks that are independent - track_mutations() { + post_template_walk() { + const script = this.ast.instance; + if (!script) return; + const component = this; + const { content } = script; const { instance_scope, instance_scope_map: map } = this; let scope = instance_scope; - walk(this.ast.instance.content, { - enter(node) { + const toRemove = []; + const remove = (parent, prop, index) => { + toRemove.unshift([parent, prop, index]); + } + + walk(content, { + enter(node, parent, prop, index) { if (map.has(node)) { scope = map.get(node); } - if (node.type === 'AssignmentExpression' || node.type === 'UpdateExpression') { - const assignee = node.type === 'AssignmentExpression' ? node.left : node.argument; - const names = extract_names(assignee); - - const deep = assignee.type === 'MemberExpression'; + if (node.type === 'ImportDeclaration') { + component.extract_imports(node); + // TODO: to use actual remove + remove(parent, prop, index); + return this.skip(); + } - names.forEach(name => { - if (scope.find_owner(name) === instance_scope) { - const variable = component.var_lookup.get(name); - variable[deep ? 'mutated' : 'reassigned'] = true; - } - }); + if (/^Export/.test(node.type)) { + const replacement = component.extract_exports(node); + if (replacement) { + this.replace(replacement); + } else { + // TODO: to use actual remove + remove(parent, prop, index); + } + return this.skip(); } + + component.warn_on_undefined_store_value_references(node, parent, scope); }, leave(node) { @@ -715,37 +756,53 @@ export default class Component { } }, }); + + for(const [parent, prop, index] of toRemove) { + if (parent) { + if (index !== null) { + parent[prop].splice(index, 1); + } else { + delete parent[prop]; + } + } + } } - warn_on_undefined_store_value_references() { - // TODO this pattern happens a lot... can we abstract it - // (or better still, do fewer AST walks)? + track_references_and_mutations() { + const script = this.ast.instance; + if (!script) return; + const component = this; - let { instance_scope: scope, instance_scope_map: map } = this; + const { content } = script; + const { instance_scope, instance_scope_map: map } = this; - walk(this.ast.instance.content, { + let scope = instance_scope; + + walk(content, { enter(node, parent) { if (map.has(node)) { scope = map.get(node); } - if ( - node.type === 'LabeledStatement' && - node.label.name === '$' && - parent.type !== 'Program' - ) { - component.warn(node as any, { - code: 'non-top-level-reactive-declaration', - message: '$: has no effect outside of the top-level', + if (node.type === 'AssignmentExpression' || node.type === 'UpdateExpression') { + const assignee = node.type === 'AssignmentExpression' ? node.left : node.argument; + const names = extract_names(assignee); + + const deep = assignee.type === 'MemberExpression'; + + names.forEach(name => { + if (scope.find_owner(name) === instance_scope) { + const variable = component.var_lookup.get(name); + variable[deep ? 'mutated' : 'reassigned'] = true; + } }); } - if (is_reference(node as Node, parent as Node)) { + if (is_used_as_reference(node, parent)) { const object = get_object(node); - const { name } = object; - - if (name[0] === '$' && !scope.has(name)) { - component.warn_if_undefined(name, object, null); + if (scope.find_owner(object.name) === instance_scope) { + const variable = component.var_lookup.get(object.name); + variable.referenced_from_script = true; } } }, @@ -758,6 +815,28 @@ export default class Component { }); } + warn_on_undefined_store_value_references(node, parent, scope) { + if ( + node.type === 'LabeledStatement' && + node.label.name === '$' && + parent.type !== 'Program' + ) { + this.warn(node as any, { + code: 'non-top-level-reactive-declaration', + message: '$: has no effect outside of the top-level', + }); + } + + if (is_reference(node as Node, parent as Node)) { + const object = get_object(node); + const { name } = object; + + if (name[0] === '$' && !scope.has(name)) { + this.warn_if_undefined(name, object, null); + } + } + } + invalidate(name, value?) { const variable = this.var_lookup.get(name); diff --git a/src/compiler/compile/utils/is_used_as_reference.ts b/src/compiler/compile/utils/is_used_as_reference.ts new file mode 100644 index 0000000000..8d55182794 --- /dev/null +++ b/src/compiler/compile/utils/is_used_as_reference.ts @@ -0,0 +1,33 @@ +import { Node } from 'estree'; +import is_reference from 'is-reference'; + +export default function is_used_as_reference( + node: Node, + parent: Node +): boolean { + if (!is_reference(node, parent)) { + return false; + } + if (!parent) { + return true; + } + + switch (parent.type) { + // disregard the `foo` in `const foo = bar` + case 'VariableDeclarator': + return node !== parent.id; + // disregard the `foo`, `bar` in `function foo(bar){}` + case 'FunctionDeclaration': + // disregard the `foo` in `import { foo } from 'foo'` + case 'ImportSpecifier': + // disregard the `foo` in `import foo from 'foo'` + case 'ImportDefaultSpecifier': + // disregard the `foo` in `import * as foo from 'foo'` + case 'ImportNamespaceSpecifier': + // disregard the `foo` in `export { foo }` + case 'ExportSpecifier': + return false; + default: + return true; + } +} diff --git a/src/compiler/interfaces.ts b/src/compiler/interfaces.ts index 1e0028e8aa..f877b93b56 100644 --- a/src/compiler/interfaces.ts +++ b/src/compiler/interfaces.ts @@ -148,7 +148,8 @@ export interface Var { module?: boolean; mutated?: boolean; reassigned?: boolean; - referenced?: boolean; + referenced?: boolean; // referenced from template scope + referenced_from_script?: boolean; // referenced from script writable?: boolean; // used internally, but not exposed diff --git a/test/validator/samples/unreferenced-variables/input.svelte b/test/validator/samples/unreferenced-variables/input.svelte new file mode 100644 index 0000000000..1180f6ef1a --- /dev/null +++ b/test/validator/samples/unreferenced-variables/input.svelte @@ -0,0 +1,21 @@ + diff --git a/test/validator/samples/unreferenced-variables/warnings.json b/test/validator/samples/unreferenced-variables/warnings.json new file mode 100644 index 0000000000..7d9e0111bf --- /dev/null +++ b/test/validator/samples/unreferenced-variables/warnings.json @@ -0,0 +1,77 @@ +[ + { + "code": "unused-export-let", + "end": { + "character": 103, + "column": 12, + "line": 8 + }, + "message": "Component has unused export property 'd'. If it is for external reference only, please consider using `export const 'd'`", + "pos": 102, + "start": { + "character": 102, + "column": 11, + "line": 8 + } + }, + { + "code": "unused-export-let", + "end": { + "character": 106, + "column": 15, + "line": 8 + }, + "message": "Component has unused export property 'e'. If it is for external reference only, please consider using `export const 'e'`", + "pos": 105, + "start": { + "character": 105, + "column": 14, + "line": 8 + } + }, + { + "code": "unused-export-let", + "end": { + "character": 130, + "column": 18, + "line": 9 + }, + "message": "Component has unused export property 'g'. If it is for external reference only, please consider using `export const 'g'`", + "pos": 125, + "start": { + "character": 125, + "column": 13, + "line": 9 + } + }, + { + "code": "unused-export-let", + "end": { + "character": 150, + "column": 18, + "line": 10 + }, + "message": "Component has unused export property 'h'. If it is for external reference only, please consider using `export const 'h'`", + "pos": 145, + "start": { + "character": 145, + "column": 13, + "line": 10 + } + }, + { + "code": "unused-export-let", + "end": { + "character": 199, + "column": 25, + "line": 12 + }, + "message": "Component has unused export property 'j'. If it is for external reference only, please consider using `export const 'j'`", + "pos": 187, + "start": { + "character": 187, + "column": 13, + "line": 12 + } + } +] diff --git a/test/vars/samples/$$props-logicless/_config.js b/test/vars/samples/$$props-logicless/_config.js index b0d76bebbd..20f9b99466 100644 --- a/test/vars/samples/$$props-logicless/_config.js +++ b/test/vars/samples/$$props-logicless/_config.js @@ -9,6 +9,7 @@ export default { mutated: false, reassigned: false, referenced: true, + referenced_from_script: false, writable: false } ]); diff --git a/test/vars/samples/$$props/_config.js b/test/vars/samples/$$props/_config.js index b0d76bebbd..20f9b99466 100644 --- a/test/vars/samples/$$props/_config.js +++ b/test/vars/samples/$$props/_config.js @@ -9,6 +9,7 @@ export default { mutated: false, reassigned: false, referenced: true, + referenced_from_script: false, writable: false } ]); diff --git a/test/vars/samples/actions/_config.js b/test/vars/samples/actions/_config.js index 6b6f8696e7..4cc2b9d18c 100644 --- a/test/vars/samples/actions/_config.js +++ b/test/vars/samples/actions/_config.js @@ -9,6 +9,7 @@ export default { name: 'hoistable_foo', reassigned: false, referenced: true, + referenced_from_script: false, writable: false }, { @@ -19,6 +20,7 @@ export default { name: 'foo', reassigned: false, referenced: true, + referenced_from_script: false, writable: true } ]); diff --git a/test/vars/samples/animations/_config.js b/test/vars/samples/animations/_config.js index 6b6f8696e7..4cc2b9d18c 100644 --- a/test/vars/samples/animations/_config.js +++ b/test/vars/samples/animations/_config.js @@ -9,6 +9,7 @@ export default { name: 'hoistable_foo', reassigned: false, referenced: true, + referenced_from_script: false, writable: false }, { @@ -19,6 +20,7 @@ export default { name: 'foo', reassigned: false, referenced: true, + referenced_from_script: false, writable: true } ]); diff --git a/test/vars/samples/component-namespaced/_config.js b/test/vars/samples/component-namespaced/_config.js index ac63873967..a801c52780 100644 --- a/test/vars/samples/component-namespaced/_config.js +++ b/test/vars/samples/component-namespaced/_config.js @@ -9,6 +9,7 @@ export default { mutated: false, reassigned: false, referenced: true, + referenced_from_script: false, writable: false } ]); diff --git a/test/vars/samples/duplicate-non-hoistable/_config.js b/test/vars/samples/duplicate-non-hoistable/_config.js index bb7f52d4db..4ebc5b00cf 100644 --- a/test/vars/samples/duplicate-non-hoistable/_config.js +++ b/test/vars/samples/duplicate-non-hoistable/_config.js @@ -9,6 +9,7 @@ export default { mutated: false, reassigned: false, referenced: true, + referenced_from_script: false, writable: true } ]); diff --git a/test/vars/samples/duplicate-vars/_config.js b/test/vars/samples/duplicate-vars/_config.js index 276ae130a1..eb10c44a9a 100644 --- a/test/vars/samples/duplicate-vars/_config.js +++ b/test/vars/samples/duplicate-vars/_config.js @@ -9,6 +9,7 @@ export default { mutated: false, reassigned: false, referenced: false, + referenced_from_script: false, writable: true }, { @@ -19,6 +20,7 @@ export default { mutated: false, reassigned: false, referenced: true, + referenced_from_script: false, writable: true } ]); diff --git a/test/vars/samples/implicit-reactive/_config.js b/test/vars/samples/implicit-reactive/_config.js index fd67e8b249..770de590e6 100644 --- a/test/vars/samples/implicit-reactive/_config.js +++ b/test/vars/samples/implicit-reactive/_config.js @@ -9,6 +9,7 @@ export default { mutated: false, reassigned: false, referenced: true, + referenced_from_script: true, writable: true }, { @@ -19,6 +20,7 @@ export default { mutated: false, reassigned: true, referenced: true, + referenced_from_script: false, writable: true } ]); diff --git a/test/vars/samples/imports/_config.js b/test/vars/samples/imports/_config.js index 90f2468b54..ecf120c7d6 100644 --- a/test/vars/samples/imports/_config.js +++ b/test/vars/samples/imports/_config.js @@ -9,6 +9,7 @@ export default { mutated: false, reassigned: false, referenced: false, + referenced_from_script: false, writable: false }, { @@ -19,6 +20,7 @@ export default { mutated: false, reassigned: false, referenced: false, + referenced_from_script: false, writable: false }, { @@ -29,6 +31,7 @@ export default { mutated: false, reassigned: false, referenced: false, + referenced_from_script: false, writable: false } ]); diff --git a/test/vars/samples/mutated-vs-reassigned-bindings/_config.js b/test/vars/samples/mutated-vs-reassigned-bindings/_config.js index 21fc14d820..ba499674d2 100644 --- a/test/vars/samples/mutated-vs-reassigned-bindings/_config.js +++ b/test/vars/samples/mutated-vs-reassigned-bindings/_config.js @@ -9,6 +9,7 @@ export default { mutated: false, reassigned: true, referenced: true, + referenced_from_script: false, writable: true }, { @@ -19,6 +20,7 @@ export default { mutated: true, reassigned: false, referenced: true, + referenced_from_script: false, writable: false } ]); diff --git a/test/vars/samples/mutated-vs-reassigned/_config.js b/test/vars/samples/mutated-vs-reassigned/_config.js index 21fc14d820..ba499674d2 100644 --- a/test/vars/samples/mutated-vs-reassigned/_config.js +++ b/test/vars/samples/mutated-vs-reassigned/_config.js @@ -9,6 +9,7 @@ export default { mutated: false, reassigned: true, referenced: true, + referenced_from_script: false, writable: true }, { @@ -19,6 +20,7 @@ export default { mutated: true, reassigned: false, referenced: true, + referenced_from_script: false, writable: false } ]); diff --git a/test/vars/samples/props/_config.js b/test/vars/samples/props/_config.js index 0c5c6f7e2a..a51c93fb03 100644 --- a/test/vars/samples/props/_config.js +++ b/test/vars/samples/props/_config.js @@ -9,6 +9,7 @@ export default { mutated: false, reassigned: false, referenced: true, + referenced_from_script: false, writable: true }, { @@ -19,6 +20,7 @@ export default { mutated: false, reassigned: false, referenced: true, + referenced_from_script: false, writable: true }, { @@ -29,6 +31,7 @@ export default { mutated: false, reassigned: false, referenced: false, + referenced_from_script: true, writable: true }, { @@ -39,6 +42,7 @@ export default { mutated: false, reassigned: true, referenced: true, + referenced_from_script: true, writable: true } ]); diff --git a/test/vars/samples/referenced-from-script/_config.js b/test/vars/samples/referenced-from-script/_config.js new file mode 100644 index 0000000000..191a52f8cc --- /dev/null +++ b/test/vars/samples/referenced-from-script/_config.js @@ -0,0 +1,160 @@ +export default { + test(assert, vars) { + assert.deepEqual(vars, [ + { + name: 'i', + export_name: null, + injected: false, + module: false, + mutated: false, + reassigned: false, + referenced: false, + writable: false, + referenced_from_script: false, + }, + { + name: 'j', + export_name: null, + injected: false, + module: false, + mutated: false, + reassigned: false, + referenced: false, + writable: false, + referenced_from_script: false, + }, + { + name: 'k', + export_name: null, + injected: false, + module: false, + mutated: false, + reassigned: false, + referenced: false, + writable: false, + referenced_from_script: false, + }, + { + name: 'a', + export_name: null, + injected: false, + module: false, + mutated: false, + reassigned: true, + referenced: false, + writable: true, + referenced_from_script: true, + }, + { + name: 'b', + export_name: null, + injected: false, + module: false, + mutated: false, + reassigned: false, + referenced: false, + writable: true, + referenced_from_script: true, + }, + { + name: 'c', + export_name: null, + injected: false, + module: false, + mutated: false, + reassigned: false, + referenced: false, + writable: true, + referenced_from_script: true, + }, + { + name: 'd', + export_name: null, + injected: false, + module: false, + mutated: false, + reassigned: false, + referenced: false, + writable: true, + referenced_from_script: true, + }, + { + name: 'e', + export_name: null, + injected: false, + module: false, + mutated: false, + reassigned: false, + referenced: false, + writable: true, + referenced_from_script: false, + }, + { + name: 'f', + export_name: 'f', + injected: false, + module: false, + mutated: false, + reassigned: false, + referenced: false, + writable: true, + referenced_from_script: false, + }, + { + name: 'g', + export_name: null, + injected: false, + module: false, + mutated: false, + reassigned: false, + referenced: false, + writable: true, + referenced_from_script: true, + }, + { + name: 'h', + export_name: null, + injected: false, + module: false, + mutated: false, + reassigned: true, + referenced: false, + writable: true, + referenced_from_script: true, + }, + { + name: 'foo', + export_name: null, + injected: false, + module: false, + mutated: false, + reassigned: false, + referenced: false, + writable: false, + referenced_from_script: false, + }, + { + name: 'l', + export_name: null, + injected: false, + module: false, + mutated: false, + reassigned: false, + referenced: false, + referenced_from_script: true, + writable: false, + }, + { + name: 'bar', + export_name: 'bar', + injected: false, + module: false, + mutated: false, + reassigned: false, + referenced: false, + writable: false, + referenced_from_script: false, + }, + ]); + }, +}; diff --git a/test/vars/samples/referenced-from-script/input.svelte b/test/vars/samples/referenced-from-script/input.svelte new file mode 100644 index 0000000000..fbec4e6cf7 --- /dev/null +++ b/test/vars/samples/referenced-from-script/input.svelte @@ -0,0 +1,16 @@ + diff --git a/test/vars/samples/store-referenced/_config.js b/test/vars/samples/store-referenced/_config.js index 632a9e5b3f..bac35d1dba 100644 --- a/test/vars/samples/store-referenced/_config.js +++ b/test/vars/samples/store-referenced/_config.js @@ -9,6 +9,7 @@ export default { mutated: false, reassigned: false, referenced: true, + referenced_from_script: false, writable: true }, { @@ -19,6 +20,7 @@ export default { mutated: true, reassigned: false, referenced: true, + referenced_from_script: false, writable: true } ]); diff --git a/test/vars/samples/store-unreferenced/_config.js b/test/vars/samples/store-unreferenced/_config.js index 86e467c859..4965e52fec 100644 --- a/test/vars/samples/store-unreferenced/_config.js +++ b/test/vars/samples/store-unreferenced/_config.js @@ -9,6 +9,7 @@ export default { mutated: false, reassigned: false, referenced: true, + referenced_from_script: true, writable: true }, { @@ -19,6 +20,7 @@ export default { mutated: true, reassigned: false, referenced: false, + referenced_from_script: false, writable: true } ]); diff --git a/test/vars/samples/template-references/_config.js b/test/vars/samples/template-references/_config.js index adacc0d2ef..674e351517 100644 --- a/test/vars/samples/template-references/_config.js +++ b/test/vars/samples/template-references/_config.js @@ -9,6 +9,7 @@ export default { name: 'Bar', reassigned: false, referenced: true, + referenced_from_script: false, writable: false, }, { @@ -19,6 +20,7 @@ export default { name: 'foo', reassigned: false, referenced: true, + referenced_from_script: false, writable: true, }, { @@ -29,6 +31,7 @@ export default { name: 'baz', reassigned: false, referenced: true, + referenced_from_script: false, writable: true, }, ]); diff --git a/test/vars/samples/transitions/_config.js b/test/vars/samples/transitions/_config.js index b2522a1b56..29a99b16cc 100644 --- a/test/vars/samples/transitions/_config.js +++ b/test/vars/samples/transitions/_config.js @@ -9,6 +9,7 @@ export default { name: 'hoistable_foo', reassigned: false, referenced: true, + referenced_from_script: false, writable: false }, { @@ -19,6 +20,7 @@ export default { name: 'hoistable_bar', reassigned: false, referenced: true, + referenced_from_script: false, writable: false }, { @@ -29,6 +31,7 @@ export default { name: 'hoistable_baz', reassigned: false, referenced: true, + referenced_from_script: false, writable: false }, { @@ -39,6 +42,7 @@ export default { name: 'foo', reassigned: false, referenced: true, + referenced_from_script: false, writable: true }, { @@ -49,6 +53,7 @@ export default { name: 'bar', reassigned: false, referenced: true, + referenced_from_script: false, writable: true }, { @@ -59,6 +64,7 @@ export default { name: 'baz', reassigned: false, referenced: true, + referenced_from_script: false, writable: true } ]); From 464868bb6c74166e530c3bafa9836c2ad20b6b9c Mon Sep 17 00:00:00 2001 From: Tan Li Hau Date: Sun, 27 Oct 2019 21:20:51 +0800 Subject: [PATCH 2/3] feat: allow innerHtml if no dynamic dependencies --- .../render_dom/wrappers/Element/index.ts | 12 ++- .../render_dom/wrappers/MustacheTag.ts | 1 - .../compile/render_dom/wrappers/shared/Tag.ts | 8 +- test/js/samples/hoisted-const/expected.js | 9 +-- test/js/samples/hoisted-let/expected.js | 9 +-- .../samples/non-mutable-reference/expected.js | 14 +--- .../samples/unchanged-expression/expected.js | 78 +++++++++++++++++++ .../samples/unchanged-expression/input.svelte | 17 ++++ test/runtime/index.js | 3 +- .../_config.js | 61 ++++++++++----- 10 files changed, 162 insertions(+), 50 deletions(-) create mode 100644 test/js/samples/unchanged-expression/expected.js create mode 100644 test/js/samples/unchanged-expression/input.svelte diff --git a/src/compiler/compile/render_dom/wrappers/Element/index.ts b/src/compiler/compile/render_dom/wrappers/Element/index.ts index 253ab107d8..ac7e80c299 100644 --- a/src/compiler/compile/render_dom/wrappers/Element/index.ts +++ b/src/compiler/compile/render_dom/wrappers/Element/index.ts @@ -6,6 +6,7 @@ import { is_void, sanitize } from '../../../../utils/names'; import FragmentWrapper from '../Fragment'; import { escape_html, string_literal } from '../../../utils/stringify'; import TextWrapper from '../Text'; +import TagWrapper from '../shared/Tag'; import fix_attribute_casing from './fix_attribute_casing'; import { b, x, p } from 'code-red'; import { namespaces } from '../../../../utils/namespaces'; @@ -849,7 +850,7 @@ export default class ElementWrapper extends Wrapper { } } -function to_html(wrappers: Array, block: Block, literal: any, state: any) { +function to_html(wrappers: Array, block: Block, literal: any, state: any) { wrappers.forEach(wrapper => { if (wrapper.node.type === 'Text') { if ((wrapper as TextWrapper).use_space()) state.quasi.value.raw += ' '; @@ -867,6 +868,15 @@ function to_html(wrappers: Array, block: Block, li .replace(/\$/g, '\\$'); } + else if (wrapper.node.type === 'MustacheTag' || wrapper.node.type === 'RawMustacheTag' ) { + literal.quasis.push(state.quasi); + literal.expressions.push(wrapper.node.expression.manipulate(block)); + state.quasi = { + type: 'TemplateElement', + value: { raw: '' } + }; + } + else if (wrapper.node.name === 'noscript') { // do nothing } diff --git a/src/compiler/compile/render_dom/wrappers/MustacheTag.ts b/src/compiler/compile/render_dom/wrappers/MustacheTag.ts index 27364f5efb..b5ac6505f8 100644 --- a/src/compiler/compile/render_dom/wrappers/MustacheTag.ts +++ b/src/compiler/compile/render_dom/wrappers/MustacheTag.ts @@ -12,7 +12,6 @@ export default class MustacheTagWrapper extends Tag { constructor(renderer: Renderer, block: Block, parent: Wrapper, node: MustacheTag | RawMustacheTag) { super(renderer, block, parent, node); - this.cannot_use_innerhtml(); } render(block: Block, parent_node: Identifier, parent_nodes: Identifier) { diff --git a/src/compiler/compile/render_dom/wrappers/shared/Tag.ts b/src/compiler/compile/render_dom/wrappers/shared/Tag.ts index 6738dd8c99..01cba3918a 100644 --- a/src/compiler/compile/render_dom/wrappers/shared/Tag.ts +++ b/src/compiler/compile/render_dom/wrappers/shared/Tag.ts @@ -11,11 +11,17 @@ export default class Tag extends Wrapper { constructor(renderer: Renderer, block: Block, parent: Wrapper, node: MustacheTag | RawMustacheTag) { super(renderer, block, parent, node); - this.cannot_use_innerhtml(); + if (!this.is_dependencies_static()) { + this.cannot_use_innerhtml(); + } block.add_dependencies(node.expression.dependencies); } + is_dependencies_static() { + return this.node.expression.contextual_dependencies.size === 0 && this.node.expression.dynamic_dependencies().length === 0; + } + rename_this_method( block: Block, update: ((value: Node) => (Node | Node[])) diff --git a/test/js/samples/hoisted-const/expected.js b/test/js/samples/hoisted-const/expected.js index af4d360d35..997b6635e6 100644 --- a/test/js/samples/hoisted-const/expected.js +++ b/test/js/samples/hoisted-const/expected.js @@ -1,28 +1,23 @@ import { SvelteComponent, - append, detach, element, init, insert, noop, - safe_not_equal, - text + safe_not_equal } from "svelte/internal"; function create_fragment(ctx) { let b; - let t_value = get_answer() + ""; - let t; return { c() { b = element("b"); - t = text(t_value); + b.innerHTML = `${get_answer()}`; }, m(target, anchor) { insert(target, b, anchor); - append(b, t); }, p: noop, i: noop, diff --git a/test/js/samples/hoisted-let/expected.js b/test/js/samples/hoisted-let/expected.js index 5392a5018c..4489e2f1af 100644 --- a/test/js/samples/hoisted-let/expected.js +++ b/test/js/samples/hoisted-let/expected.js @@ -1,28 +1,23 @@ import { SvelteComponent, - append, detach, element, init, insert, noop, - safe_not_equal, - text + safe_not_equal } from "svelte/internal"; function create_fragment(ctx) { let b; - let t_value = get_answer() + ""; - let t; return { c() { b = element("b"); - t = text(t_value); + b.innerHTML = `${get_answer()}`; }, m(target, anchor) { insert(target, b, anchor); - append(b, t); }, p: noop, i: noop, diff --git a/test/js/samples/non-mutable-reference/expected.js b/test/js/samples/non-mutable-reference/expected.js index 246850aaf4..c385ad9fe5 100644 --- a/test/js/samples/non-mutable-reference/expected.js +++ b/test/js/samples/non-mutable-reference/expected.js @@ -1,33 +1,23 @@ import { SvelteComponent, - append, detach, element, init, insert, noop, - safe_not_equal, - text + safe_not_equal } from "svelte/internal"; function create_fragment(ctx) { let h1; - let t0; - let t1; - let t2; return { c() { h1 = element("h1"); - t0 = text("Hello "); - t1 = text(name); - t2 = text("!"); + h1.innerHTML = `Hello ${name}!`; }, m(target, anchor) { insert(target, h1, anchor); - append(h1, t0); - append(h1, t1); - append(h1, t2); }, p: noop, i: noop, diff --git a/test/js/samples/unchanged-expression/expected.js b/test/js/samples/unchanged-expression/expected.js new file mode 100644 index 0000000000..ec16490f85 --- /dev/null +++ b/test/js/samples/unchanged-expression/expected.js @@ -0,0 +1,78 @@ +import { + SvelteComponent, + append, + detach, + element, + init, + insert, + noop, + safe_not_equal, + set_data, + space, + text +} from "svelte/internal"; + +function create_fragment(ctx) { + let div0; + let t7; + let div1; + let p3; + let t8; + let t9; + + return { + c() { + div0 = element("div"); + + div0.innerHTML = `

Hello world

+

Hello ${world1}

+

Hello ${world2}

`; + + t7 = space(); + div1 = element("div"); + p3 = element("p"); + t8 = text("Hello "); + t9 = text(ctx.world3); + }, + m(target, anchor) { + insert(target, div0, anchor); + insert(target, t7, anchor); + insert(target, div1, anchor); + append(div1, p3); + append(p3, t8); + append(p3, t9); + }, + p(changed, ctx) { + if (changed.world3) set_data(t9, ctx.world3); + }, + i: noop, + o: noop, + d(detaching) { + if (detaching) detach(div0); + if (detaching) detach(t7); + if (detaching) detach(div1); + } + }; +} + +let world1 = "world"; +let world2 = "world"; + +function instance($$self, $$props, $$invalidate) { + const world3 = "world"; + + function foo() { + $$invalidate("world3", world3 = "svelte"); + } + + return { world3 }; +} + +class Component extends SvelteComponent { + constructor(options) { + super(); + init(this, options, instance, create_fragment, safe_not_equal, []); + } +} + +export default Component; \ No newline at end of file diff --git a/test/js/samples/unchanged-expression/input.svelte b/test/js/samples/unchanged-expression/input.svelte new file mode 100644 index 0000000000..057201358c --- /dev/null +++ b/test/js/samples/unchanged-expression/input.svelte @@ -0,0 +1,17 @@ + +
+

Hello world

+

Hello {world1}

+

Hello {world2}

+
+
+

Hello {world3}

+
diff --git a/test/runtime/index.js b/test/runtime/index.js index 397cfef172..5955b06c0b 100644 --- a/test/runtime/index.js +++ b/test/runtime/index.js @@ -166,7 +166,8 @@ describe("runtime", () => { mod, target, window, - raf + raf, + compileOptions })).then(() => { component.$destroy(); diff --git a/test/runtime/samples/lifecycle-render-order-for-children/_config.js b/test/runtime/samples/lifecycle-render-order-for-children/_config.js index a1d35d7aa8..033b593aea 100644 --- a/test/runtime/samples/lifecycle-render-order-for-children/_config.js +++ b/test/runtime/samples/lifecycle-render-order-for-children/_config.js @@ -3,26 +3,47 @@ import order from './order.js'; export default { skip_if_ssr: true, - test({ assert, component, target }) { - assert.deepEqual(order, [ - '0: beforeUpdate', - '0: render', - '1: beforeUpdate', - '1: render', - '2: beforeUpdate', - '2: render', - '3: beforeUpdate', - '3: render', - '1: onMount', - '1: afterUpdate', - '2: onMount', - '2: afterUpdate', - '3: onMount', - '3: afterUpdate', - '0: onMount', - '0: afterUpdate' - ]); + test({ assert, component, target, compileOptions }) { + if (compileOptions.hydratable) { + assert.deepEqual(order, [ + '0: beforeUpdate', + '0: render', + '1: beforeUpdate', + '1: render', + '2: beforeUpdate', + '2: render', + '3: beforeUpdate', + '3: render', + '1: onMount', + '1: afterUpdate', + '2: onMount', + '2: afterUpdate', + '3: onMount', + '3: afterUpdate', + '0: onMount', + '0: afterUpdate', + ]); + } else { + assert.deepEqual(order, [ + '0: beforeUpdate', + '0: render', + '1: beforeUpdate', + '2: beforeUpdate', + '3: beforeUpdate', + '1: render', + '2: render', + '3: render', + '1: onMount', + '1: afterUpdate', + '2: onMount', + '2: afterUpdate', + '3: onMount', + '3: afterUpdate', + '0: onMount', + '0: afterUpdate', + ]); + } order.length = 0; - } + }, }; From 60b240bf69721019cb81ce827bfc96b86fe4e0b8 Mon Sep 17 00:00:00 2001 From: Tan Li Hau Date: Sun, 27 Oct 2019 21:29:45 +0800 Subject: [PATCH 3/3] fix lint --- src/compiler/compile/Component.ts | 6 +++--- src/compiler/compile/utils/is_used_as_reference.ts | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/compiler/compile/Component.ts b/src/compiler/compile/Component.ts index cc3bf2e308..4d8abadf34 100644 --- a/src/compiler/compile/Component.ts +++ b/src/compiler/compile/Component.ts @@ -680,7 +680,7 @@ export default class Component { const { body } = script.content; let i = body.length; - while(--i >= 0) { + while (--i >= 0) { const node = body[i]; if (node.type === 'ImportDeclaration') { this.extract_imports(node); @@ -721,7 +721,7 @@ export default class Component { const toRemove = []; const remove = (parent, prop, index) => { toRemove.unshift([parent, prop, index]); - } + }; walk(content, { enter(node, parent, prop, index) { @@ -757,7 +757,7 @@ export default class Component { }, }); - for(const [parent, prop, index] of toRemove) { + for (const [parent, prop, index] of toRemove) { if (parent) { if (index !== null) { parent[prop].splice(index, 1); diff --git a/src/compiler/compile/utils/is_used_as_reference.ts b/src/compiler/compile/utils/is_used_as_reference.ts index 8d55182794..3ea3d2f242 100644 --- a/src/compiler/compile/utils/is_used_as_reference.ts +++ b/src/compiler/compile/utils/is_used_as_reference.ts @@ -12,6 +12,7 @@ export default function is_used_as_reference( return true; } + /* eslint-disable no-fallthrough */ switch (parent.type) { // disregard the `foo` in `const foo = bar` case 'VariableDeclarator':