From 6b1067e8c1a3bdc1331b70a8d08f15c6fbd2fb1e Mon Sep 17 00:00:00 2001 From: Richard Harris Date: Thu, 11 Apr 2019 23:29:02 -0400 Subject: [PATCH] allow reactive declarations without dependencies - fixes #2285 --- src/compile/Component.ts | 7 --- src/compile/render-dom/index.ts | 52 +++++++++++-------- .../expected.js | 4 +- .../expected.js | 8 +-- .../expected.js | 4 +- .../runtime/samples/props-reactive/_config.js | 2 - .../_config.js | 12 +++++ .../main.svelte | 10 ++++ .../reactive-declaration-no-deps/errors.json | 7 --- .../reactive-declaration-no-deps/input.svelte | 3 -- 10 files changed, 56 insertions(+), 53 deletions(-) create mode 100644 test/runtime/samples/reactive-values-no-dependencies/_config.js create mode 100644 test/runtime/samples/reactive-values-no-dependencies/main.svelte delete mode 100644 test/validator/samples/reactive-declaration-no-deps/errors.json delete mode 100644 test/validator/samples/reactive-declaration-no-deps/input.svelte diff --git a/src/compile/Component.ts b/src/compile/Component.ts index e55af65de6..78715415f2 100644 --- a/src/compile/Component.ts +++ b/src/compile/Component.ts @@ -1134,13 +1134,6 @@ export default class Component { seen.add(declaration); - if (declaration.dependencies.size === 0) { - this.error(declaration.node, { - code: 'invalid-reactive-declaration', - message: 'Invalid reactive declaration — must depend on local state' - }); - } - declaration.dependencies.forEach(name => { if (declaration.assignees.has(name)) return; const earlier_declarations = lookup.get(name); diff --git a/src/compile/render-dom/index.ts b/src/compile/render-dom/index.ts index 363d807916..6aa36d88f7 100644 --- a/src/compile/render-dom/index.ts +++ b/src/compile/render-dom/index.ts @@ -347,28 +347,34 @@ export default function dom( .map(({ name }) => `$$self.$$.on_destroy.push(() => $$unsubscribe_${name.slice(1)}());`); if (has_definition) { - const reactive_declarations = component.reactive_declarations.map(d => { - const condition = Array.from(d.dependencies) - .filter(n => { - if (n === '$$props') return false; - const variable = component.var_lookup.get(n); - return variable && variable.writable; - }) - .map(n => `$$dirty.${n}`).join(' || '); - - const snippet = d.node.body.type === 'BlockStatement' - ? `[✂${d.node.body.start}-${d.node.end}✂]` - : deindent` - { - [✂${d.node.body.start}-${d.node.end}✂] - }`; - - return condition - ? deindent` - if (${condition}) ${snippet}` - : deindent` - ${snippet}` - }); + const reactive_declarations = []; + const fixed_reactive_declarations = []; // not really 'reactive' but whatever + + component.reactive_declarations + .forEach(d => { + let uses_props; + + const condition = Array.from(d.dependencies) + .filter(n => { + if (n === '$$props') { + uses_props = true; + return false; + } + + const variable = component.var_lookup.get(n); + return variable && variable.writable; + }) + .map(n => `$$dirty.${n}`).join(' || '); + + let snippet = `[✂${d.node.body.start}-${d.node.end}✂]`; + if (condition) snippet = `if (${condition}) { ${snippet} }`; + + if (condition || uses_props) { + reactive_declarations.push(snippet); + } else { + fixed_reactive_declarations.push(snippet); + } + }); const injected = Array.from(component.injected_reactive_declaration_vars).filter(name => { const variable = component.var_lookup.get(name); @@ -410,6 +416,8 @@ export default function dom( $$self.$$.update = ($$dirty = { ${Array.from(all_reactive_dependencies).map(n => `${n}: 1`).join(', ')} }) => { ${reactive_declarations} }; + + ${fixed_reactive_declarations} `} return ${stringify_props(filtered_declarations)}; diff --git a/test/js/samples/dev-warning-missing-data-computed/expected.js b/test/js/samples/dev-warning-missing-data-computed/expected.js index ff1f48e2be..1585e1ea27 100644 --- a/test/js/samples/dev-warning-missing-data-computed/expected.js +++ b/test/js/samples/dev-warning-missing-data-computed/expected.js @@ -70,9 +70,7 @@ function instance($$self, $$props, $$invalidate) { }; $$self.$$.update = ($$dirty = { foo: 1 }) => { - if ($$dirty.foo) { - bar = foo * 2; $$invalidate('bar', bar); - } + if ($$dirty.foo) { bar = foo * 2; $$invalidate('bar', bar); } }; return { foo, bar }; diff --git a/test/js/samples/reactive-values-non-topologically-ordered/expected.js b/test/js/samples/reactive-values-non-topologically-ordered/expected.js index a0a5363d6f..b0bd493d0f 100644 --- a/test/js/samples/reactive-values-non-topologically-ordered/expected.js +++ b/test/js/samples/reactive-values-non-topologically-ordered/expected.js @@ -28,12 +28,8 @@ function instance($$self, $$props, $$invalidate) { }; $$self.$$.update = ($$dirty = { x: 1, b: 1 }) => { - if ($$dirty.x) { - b = x; $$invalidate('b', b); - } - if ($$dirty.b) { - a = b; $$invalidate('a', a); - } + if ($$dirty.x) { b = x; $$invalidate('b', b); } + if ($$dirty.b) { a = b; $$invalidate('a', a); } }; return { x }; diff --git a/test/js/samples/reactive-values-non-writable-dependencies/expected.js b/test/js/samples/reactive-values-non-writable-dependencies/expected.js index 2e4507222f..fc6f059d4e 100644 --- a/test/js/samples/reactive-values-non-writable-dependencies/expected.js +++ b/test/js/samples/reactive-values-non-writable-dependencies/expected.js @@ -27,9 +27,7 @@ function instance($$self, $$props, $$invalidate) { let max; $$self.$$.update = ($$dirty = { a: 1, b: 1 }) => { - if ($$dirty.a || $$dirty.b) { - max = Math.max(a, b); $$invalidate('max', max); - } + if ($$dirty.a || $$dirty.b) { max = Math.max(a, b); $$invalidate('max', max); } }; return {}; diff --git a/test/runtime/samples/props-reactive/_config.js b/test/runtime/samples/props-reactive/_config.js index 49e18546c7..a44687069e 100644 --- a/test/runtime/samples/props-reactive/_config.js +++ b/test/runtime/samples/props-reactive/_config.js @@ -1,6 +1,4 @@ export default { - show: 1, - props: { a: 1, b: 2, diff --git a/test/runtime/samples/reactive-values-no-dependencies/_config.js b/test/runtime/samples/reactive-values-no-dependencies/_config.js new file mode 100644 index 0000000000..71a63c9189 --- /dev/null +++ b/test/runtime/samples/reactive-values-no-dependencies/_config.js @@ -0,0 +1,12 @@ +export default { + html: ` +

10 - 90

+ `, + + test({ assert, component, target }) { + component.width = 50; + assert.htmlEqual(target.innerHTML, ` +

10 - 40

+ `); + } +}; diff --git a/test/runtime/samples/reactive-values-no-dependencies/main.svelte b/test/runtime/samples/reactive-values-no-dependencies/main.svelte new file mode 100644 index 0000000000..bf54b8e241 --- /dev/null +++ b/test/runtime/samples/reactive-values-no-dependencies/main.svelte @@ -0,0 +1,10 @@ + + +

{x1} - {x2}

\ No newline at end of file diff --git a/test/validator/samples/reactive-declaration-no-deps/errors.json b/test/validator/samples/reactive-declaration-no-deps/errors.json deleted file mode 100644 index 0063d4005e..0000000000 --- a/test/validator/samples/reactive-declaration-no-deps/errors.json +++ /dev/null @@ -1,7 +0,0 @@ -[{ - "message": "Invalid reactive declaration — must depend on local state", - "code": "invalid-reactive-declaration", - "start": { "line": 2, "column": 1, "character": 10 }, - "end": { "line": 2, "column": 23, "character": 32 }, - "pos": 10 -}] diff --git a/test/validator/samples/reactive-declaration-no-deps/input.svelte b/test/validator/samples/reactive-declaration-no-deps/input.svelte deleted file mode 100644 index c334f92ab5..0000000000 --- a/test/validator/samples/reactive-declaration-no-deps/input.svelte +++ /dev/null @@ -1,3 +0,0 @@ -