From 6d2ab63112317fb2a442ef0fb8802bffa270db7f Mon Sep 17 00:00:00 2001 From: baseballyama Date: Fri, 4 Feb 2022 14:56:39 +0900 Subject: [PATCH 1/3] Fix rvalue error when using arrow functions in {@const} --- .../compile/nodes/shared/Expression.ts | 28 ++++++++++++++-- .../samples/const-tag-each-const/_config.js | 29 ++++++++++++++++ .../samples/const-tag-each-const/main.svelte | 26 +++++++++++++++ .../_config.js | 29 ++++++++++++++++ .../main.svelte | 19 +++++++++++ .../_config.js | 32 ++++++++++++++++++ .../main.svelte | 33 +++++++++++++++++++ .../_config.js | 29 ++++++++++++++++ .../main.svelte | 25 ++++++++++++++ .../const-tag-each-function/_config.js | 29 ++++++++++++++++ .../const-tag-each-function/main.svelte | 26 +++++++++++++++ 11 files changed, 302 insertions(+), 3 deletions(-) create mode 100644 test/runtime/samples/const-tag-each-const/_config.js create mode 100644 test/runtime/samples/const-tag-each-const/main.svelte create mode 100644 test/runtime/samples/const-tag-each-duplicated-variable1/_config.js create mode 100644 test/runtime/samples/const-tag-each-duplicated-variable1/main.svelte create mode 100644 test/runtime/samples/const-tag-each-duplicated-variable2/_config.js create mode 100644 test/runtime/samples/const-tag-each-duplicated-variable2/main.svelte create mode 100644 test/runtime/samples/const-tag-each-duplicated-variable3/_config.js create mode 100644 test/runtime/samples/const-tag-each-duplicated-variable3/main.svelte create mode 100644 test/runtime/samples/const-tag-each-function/_config.js create mode 100644 test/runtime/samples/const-tag-each-function/main.svelte diff --git a/src/compiler/compile/nodes/shared/Expression.ts b/src/compiler/compile/nodes/shared/Expression.ts index 98fb2f1e3ac..c6f249ef295 100644 --- a/src/compiler/compile/nodes/shared/Expression.ts +++ b/src/compiler/compile/nodes/shared/Expression.ts @@ -10,7 +10,7 @@ import Block from '../../render_dom/Block'; import is_dynamic from '../../render_dom/wrappers/shared/is_dynamic'; import { b } from 'code-red'; import { invalidate } from '../../render_dom/invalidate'; -import { Node, FunctionExpression, Identifier } from 'estree'; +import { BaseNode, Node, BaseFunction, FunctionExpression, Identifier, Pattern, MemberExpression } from 'estree'; import { INode } from '../interfaces'; import { is_reserved_keyword } from '../../utils/reserved_keywords'; import replace_object from '../../utils/replace_object'; @@ -254,11 +254,33 @@ export default class Expression { const declaration = b`const ${id} = ${node}`; if (owner.type === 'ConstTag') { + const param_names_stack: string[][] = []; + const is_base_function = (node: BaseNode): node is BaseFunction => 'params' in node; + const is_member_expression = (node: BaseNode): node is MemberExpression => 'computed' in node; + const push_params = (params: Pattern[]) => { + const param_names: string[] = []; + params.forEach((param => { + walk(param, { + enter(node: Node) { + if (node.type === 'Identifier') param_names.push(node.name); + } + }); + })); + param_names_stack.push(param_names); + }; + const is_context = (node: Identifier, parent: Node, key: string): boolean => { + if (key === 'params' || (key === 'property' && is_member_expression(parent) && !parent.computed)) return false; + return !param_names_stack.some((param_names) => param_names.includes(node.name)); + }; walk(node, { - enter(node: Node) { - if (node.type === 'Identifier') { + enter(node: Node, parent: Node, key: string) { + if (is_base_function(node)) push_params(node.params); + if (node.type === 'Identifier' && is_context(node, parent, key)) { this.replace(block.renderer.reference(node, ctx)); } + }, + leave(node: Node) { + if (is_base_function(node)) param_names_stack.pop(); } }); } else if (dependencies.size === 0 && contextual_dependencies.size === 0) { diff --git a/test/runtime/samples/const-tag-each-const/_config.js b/test/runtime/samples/const-tag-each-const/_config.js new file mode 100644 index 00000000000..cde4226a253 --- /dev/null +++ b/test/runtime/samples/const-tag-each-const/_config.js @@ -0,0 +1,29 @@ +export default { + html: ` +

0

+

bar: 1,2,3,1,1,2,3,2, num: 1

+

bar: 0,2,4,1,0,2,4,2, num: 2

+ `, + async test({ component, target, assert }) { + assert.htmlEqual( + target.innerHTML, + ` +

0

+

bar: 1,2,3,1,1,2,3,2, num: 1

+

bar: 0,2,4,1,0,2,4,2, num: 2

+ ` + ); + + component.nums = [1, 2, 3]; + + assert.htmlEqual( + target.innerHTML, + ` +

0

+

bar: 1,2,3,1,1,2,3,2,1,2,3,3, num: 1

+

bar: 0,2,4,1,0,2,4,2,0,2,4,3, num: 2

+

bar: -100,0,100,1,-100,0,100,2,-100,0,100,3, num: 3

+ ` + ); + } +}; diff --git a/test/runtime/samples/const-tag-each-const/main.svelte b/test/runtime/samples/const-tag-each-const/main.svelte new file mode 100644 index 00000000000..182ea3cfc64 --- /dev/null +++ b/test/runtime/samples/const-tag-each-const/main.svelte @@ -0,0 +1,26 @@ + + +

{foo}

+{#each nums as num, index} + {@const bar = nums.map((num) => { + const func = (foos, num) => { + return [...foos.map((foo) => foo), num]; + } + return func(foos[index].nums, num); + })} +

bar: {bar}, num: {num}

+{/each} diff --git a/test/runtime/samples/const-tag-each-duplicated-variable1/_config.js b/test/runtime/samples/const-tag-each-duplicated-variable1/_config.js new file mode 100644 index 00000000000..1f3e5856f28 --- /dev/null +++ b/test/runtime/samples/const-tag-each-duplicated-variable1/_config.js @@ -0,0 +1,29 @@ +export default { + html: ` +

bar: 1,2,3,0,2,4,-100,0,100, num: 1

+

bar: 1,2,3,0,2,4,-100,0,100, num: 2

+

bar: 1,2,3,0,2,4,-100,0,100, num: 3

+ `, + async test({ component, target, assert }) { + assert.htmlEqual( + target.innerHTML, + ` +

bar: 1,2,3,0,2,4,-100,0,100, num: 1

+

bar: 1,2,3,0,2,4,-100,0,100, num: 2

+

bar: 1,2,3,0,2,4,-100,0,100, num: 3

+ ` + ); + + component.nums = [1, 2, 3, 4]; + + assert.htmlEqual( + target.innerHTML, + ` +

bar: 1,2,3,0,2,4,-100,0,100, num: 1

+

bar: 1,2,3,0,2,4,-100,0,100, num: 2

+

bar: 1,2,3,0,2,4,-100,0,100, num: 3

+

bar: 1,2,3,0,2,4,-100,0,100, num: 4

+ ` + ); + } +}; diff --git a/test/runtime/samples/const-tag-each-duplicated-variable1/main.svelte b/test/runtime/samples/const-tag-each-duplicated-variable1/main.svelte new file mode 100644 index 00000000000..00c53dd4ff6 --- /dev/null +++ b/test/runtime/samples/const-tag-each-duplicated-variable1/main.svelte @@ -0,0 +1,19 @@ + + +{#each nums as num} + {@const bar = foos.map((foos) => foos.nums)} +

bar: {bar}, num: {num}

+{/each} diff --git a/test/runtime/samples/const-tag-each-duplicated-variable2/_config.js b/test/runtime/samples/const-tag-each-duplicated-variable2/_config.js new file mode 100644 index 00000000000..16f48218f18 --- /dev/null +++ b/test/runtime/samples/const-tag-each-duplicated-variable2/_config.js @@ -0,0 +1,32 @@ +export default { + html: ` +

foo: dummy-foo, num: dummy-num

+

bar: 1,2,3,2,, num: 1

+

bar: 1,2,3,2,, num: 2

+

bar: 1,2,3,2,, num: 3

+ `, + async test({ component, target, assert }) { + assert.htmlEqual( + target.innerHTML, + ` +

foo: dummy-foo, num: dummy-num

+

bar: 1,2,3,2,, num: 1

+

bar: 1,2,3,2,, num: 2

+

bar: 1,2,3,2,, num: 3

+ ` + ); + + component.nums = [1, 2, 3, 4]; + + assert.htmlEqual( + target.innerHTML, + ` +

foo: dummy-foo, num: dummy-num

+

bar: 1,2,3,2,4,, num: 1

+

bar: 1,2,3,2,4,, num: 2

+

bar: 1,2,3,2,4,, num: 3

+

bar: 1,2,3,2,4,, num: 4

+ ` + ); + } +}; diff --git a/test/runtime/samples/const-tag-each-duplicated-variable2/main.svelte b/test/runtime/samples/const-tag-each-duplicated-variable2/main.svelte new file mode 100644 index 00000000000..1f7afd92fd6 --- /dev/null +++ b/test/runtime/samples/const-tag-each-duplicated-variable2/main.svelte @@ -0,0 +1,33 @@ + + +

foo: {foo}, num: {num}

+{#each nums as num} + {@const bar = foos.map((foo) => + foo.nums.filter((num) => { + if (Object.keys($$slots).length) { + return false; + } else if (Object.keys(foo).length) { + return nums.includes(num) || default_nums.includes(num); + } else { + return false; + } + }) || num + )} +

bar: {bar}, num: {num}

+{/each} diff --git a/test/runtime/samples/const-tag-each-duplicated-variable3/_config.js b/test/runtime/samples/const-tag-each-duplicated-variable3/_config.js new file mode 100644 index 00000000000..cde4226a253 --- /dev/null +++ b/test/runtime/samples/const-tag-each-duplicated-variable3/_config.js @@ -0,0 +1,29 @@ +export default { + html: ` +

0

+

bar: 1,2,3,1,1,2,3,2, num: 1

+

bar: 0,2,4,1,0,2,4,2, num: 2

+ `, + async test({ component, target, assert }) { + assert.htmlEqual( + target.innerHTML, + ` +

0

+

bar: 1,2,3,1,1,2,3,2, num: 1

+

bar: 0,2,4,1,0,2,4,2, num: 2

+ ` + ); + + component.nums = [1, 2, 3]; + + assert.htmlEqual( + target.innerHTML, + ` +

0

+

bar: 1,2,3,1,1,2,3,2,1,2,3,3, num: 1

+

bar: 0,2,4,1,0,2,4,2,0,2,4,3, num: 2

+

bar: -100,0,100,1,-100,0,100,2,-100,0,100,3, num: 3

+ ` + ); + } +}; diff --git a/test/runtime/samples/const-tag-each-duplicated-variable3/main.svelte b/test/runtime/samples/const-tag-each-duplicated-variable3/main.svelte new file mode 100644 index 00000000000..f559627d239 --- /dev/null +++ b/test/runtime/samples/const-tag-each-duplicated-variable3/main.svelte @@ -0,0 +1,25 @@ + + +

{foo}

+{#each nums as num, index} + {@const bar = nums.map((num) => { + return (function (foos, num) { + return [...foos.map((foo) => foo), num]; + })(foos[index].nums, num); + })} +

bar: {bar}, num: {num}

+{/each} diff --git a/test/runtime/samples/const-tag-each-function/_config.js b/test/runtime/samples/const-tag-each-function/_config.js new file mode 100644 index 00000000000..cde4226a253 --- /dev/null +++ b/test/runtime/samples/const-tag-each-function/_config.js @@ -0,0 +1,29 @@ +export default { + html: ` +

0

+

bar: 1,2,3,1,1,2,3,2, num: 1

+

bar: 0,2,4,1,0,2,4,2, num: 2

+ `, + async test({ component, target, assert }) { + assert.htmlEqual( + target.innerHTML, + ` +

0

+

bar: 1,2,3,1,1,2,3,2, num: 1

+

bar: 0,2,4,1,0,2,4,2, num: 2

+ ` + ); + + component.nums = [1, 2, 3]; + + assert.htmlEqual( + target.innerHTML, + ` +

0

+

bar: 1,2,3,1,1,2,3,2,1,2,3,3, num: 1

+

bar: 0,2,4,1,0,2,4,2,0,2,4,3, num: 2

+

bar: -100,0,100,1,-100,0,100,2,-100,0,100,3, num: 3

+ ` + ); + } +}; diff --git a/test/runtime/samples/const-tag-each-function/main.svelte b/test/runtime/samples/const-tag-each-function/main.svelte new file mode 100644 index 00000000000..2e027c66208 --- /dev/null +++ b/test/runtime/samples/const-tag-each-function/main.svelte @@ -0,0 +1,26 @@ + + +

{foo}

+{#each nums as num, index} + {@const bar = nums.map((num) => { + function func(foos, num) { + return [...foos.map((foo) => foo), num]; + } + return func(foos[index].nums, num); + })} +

bar: {bar}, num: {num}

+{/each} From b488966b50a0d6bd6075defc850f7d83ab33fd7d Mon Sep 17 00:00:00 2001 From: tanhauhau Date: Sat, 9 Apr 2022 00:35:18 +0800 Subject: [PATCH 2/3] fix const tag expression replacement within scope --- .../compile/nodes/shared/Expression.ts | 28 ++++--------------- 1 file changed, 6 insertions(+), 22 deletions(-) diff --git a/src/compiler/compile/nodes/shared/Expression.ts b/src/compiler/compile/nodes/shared/Expression.ts index c6f249ef295..6781e54b097 100644 --- a/src/compiler/compile/nodes/shared/Expression.ts +++ b/src/compiler/compile/nodes/shared/Expression.ts @@ -254,33 +254,17 @@ export default class Expression { const declaration = b`const ${id} = ${node}`; if (owner.type === 'ConstTag') { - const param_names_stack: string[][] = []; - const is_base_function = (node: BaseNode): node is BaseFunction => 'params' in node; - const is_member_expression = (node: BaseNode): node is MemberExpression => 'computed' in node; - const push_params = (params: Pattern[]) => { - const param_names: string[] = []; - params.forEach((param => { - walk(param, { - enter(node: Node) { - if (node.type === 'Identifier') param_names.push(node.name); - } - }); - })); - param_names_stack.push(param_names); - }; - const is_context = (node: Identifier, parent: Node, key: string): boolean => { - if (key === 'params' || (key === 'property' && is_member_expression(parent) && !parent.computed)) return false; - return !param_names_stack.some((param_names) => param_names.includes(node.name)); - }; + let child_scope = scope; walk(node, { - enter(node: Node, parent: Node, key: string) { - if (is_base_function(node)) push_params(node.params); - if (node.type === 'Identifier' && is_context(node, parent, key)) { + enter(node: Node, parent: any) { + if (map.has(node)) child_scope = map.get(node); + if (node.type === 'Identifier' && is_reference(node, parent)) { + if (child_scope.has(node.name)) return; this.replace(block.renderer.reference(node, ctx)); } }, leave(node: Node) { - if (is_base_function(node)) param_names_stack.pop(); + if (map.has(node)) child_scope = child_scope.parent; } }); } else if (dependencies.size === 0 && contextual_dependencies.size === 0) { From c88caf43bf51f53d44bf07e85a6af5cc7936ae80 Mon Sep 17 00:00:00 2001 From: tanhauhau Date: Sat, 9 Apr 2022 00:46:06 +0800 Subject: [PATCH 3/3] add another test --- .../compile/nodes/shared/Expression.ts | 2 +- .../samples/const-tag-shadow-2/_config.js | 37 +++++++++++++++++++ .../samples/const-tag-shadow-2/main.svelte | 15 ++++++++ 3 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 test/runtime/samples/const-tag-shadow-2/_config.js create mode 100644 test/runtime/samples/const-tag-shadow-2/main.svelte diff --git a/src/compiler/compile/nodes/shared/Expression.ts b/src/compiler/compile/nodes/shared/Expression.ts index 6781e54b097..751b7395643 100644 --- a/src/compiler/compile/nodes/shared/Expression.ts +++ b/src/compiler/compile/nodes/shared/Expression.ts @@ -10,7 +10,7 @@ import Block from '../../render_dom/Block'; import is_dynamic from '../../render_dom/wrappers/shared/is_dynamic'; import { b } from 'code-red'; import { invalidate } from '../../render_dom/invalidate'; -import { BaseNode, Node, BaseFunction, FunctionExpression, Identifier, Pattern, MemberExpression } from 'estree'; +import { Node, FunctionExpression, Identifier } from 'estree'; import { INode } from '../interfaces'; import { is_reserved_keyword } from '../../utils/reserved_keywords'; import replace_object from '../../utils/replace_object'; diff --git a/test/runtime/samples/const-tag-shadow-2/_config.js b/test/runtime/samples/const-tag-shadow-2/_config.js new file mode 100644 index 00000000000..9ad4aed48cd --- /dev/null +++ b/test/runtime/samples/const-tag-shadow-2/_config.js @@ -0,0 +1,37 @@ +export default { + html: ` +

1

+

3,6,9

+

2

+

3,6,9

+

3

+

3,6,9

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

1

+

5,10,15

+

2

+

5,10,15

+

3

+

5,10,15

+ ` + ); + + component.array = [3, 4, 5]; + assert.htmlEqual( + target.innerHTML, + ` +

3

+

15,20,25

+

4

+

15,20,25

+

5

+

15,20,25

+ ` + ); + } +}; diff --git a/test/runtime/samples/const-tag-shadow-2/main.svelte b/test/runtime/samples/const-tag-shadow-2/main.svelte new file mode 100644 index 00000000000..c3bcb2f6051 --- /dev/null +++ b/test/runtime/samples/const-tag-shadow-2/main.svelte @@ -0,0 +1,15 @@ + + +{#each array as item} +

{foo(item)}

+ {@const bar = array.map((item) => { + const bar = baz; + const foo = (item) => item * bar; + return foo(item); + })} +

{bar}

+{/each}