Skip to content

Commit

Permalink
deconflict bind:this variable (#4949)
Browse files Browse the repository at this point in the history
  • Loading branch information
tanhauhau authored Jun 10, 2020
1 parent 38de3b2 commit 7dfd9e9
Show file tree
Hide file tree
Showing 11 changed files with 62 additions and 18 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
## Unreleased

* Fix `bind:group` inside `{#each}` ([#3243](https://github.com/sveltejs/svelte/issues/3243))
* Deconflict `bind:this` variable ([#4636](https://github.com/sveltejs/svelte/issues/4636))

## 3.23.1

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ export default class InlineComponentWrapper extends Wrapper {
const { component } = renderer;

const name = this.var;
block.add_variable(name);

const component_opts = x`{}` as ObjectExpression;

Expand Down Expand Up @@ -411,7 +412,7 @@ export default class InlineComponentWrapper extends Wrapper {
}
if (${switch_value}) {
var ${name} = new ${switch_value}(${switch_props}(#ctx));
${name} = new ${switch_value}(${switch_props}(#ctx));
${munged_bindings}
${munged_handlers}
Expand Down Expand Up @@ -489,7 +490,7 @@ export default class InlineComponentWrapper extends Wrapper {
${(this.node.attributes.length > 0 || this.node.bindings.length > 0) && b`
${props && b`let ${props} = ${attribute_object};`}`}
${statements}
const ${name} = new ${expression}(${component_opts});
${name} = new ${expression}(${component_opts});
${munged_bindings}
${munged_handlers}
Expand Down
28 changes: 21 additions & 7 deletions src/compiler/compile/render_dom/wrappers/shared/bind_this.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import Component from '../../../Component';
import Block from '../../Block';
import BindingWrapper from '../Element/Binding';
import { Identifier } from 'estree';
import { compare_node } from '../../../utils/compare_node';

export default function bind_this(component: Component, block: Block, binding: BindingWrapper, variable: Identifier) {
const fn = component.get_unique_name(`${variable.name}_binding`);
Expand Down Expand Up @@ -35,13 +36,26 @@ export default function bind_this(component: Component, block: Block, binding: B
}
`);

const alias_map = new Map();
const args = [];
for (const id of params) {
args.push(id);
for (let id of params) {
const value = block.renderer.reference(id.name);
let found = false;
if (block.variables.has(id.name)) {
if (block.renderer.context_lookup.get(id.name).is_contextual) continue;
let alias = id.name;
for (
let i = 1;
block.variables.has(alias) && !compare_node(block.variables.get(alias).init, value);
alias = `${id.name}_${i++}`
);
alias_map.set(alias, id.name);
id = { type: 'Identifier', name: alias };
found = block.variables.has(alias);
}
args.push(id);
if (!found) {
block.add_variable(id, value);
}
block.add_variable(id, block.renderer.reference(id.name));
}

const assign = block.get_unique_name(`assign_${variable.name}`);
Expand All @@ -52,8 +66,8 @@ export default function bind_this(component: Component, block: Block, binding: B
const ${unassign} = () => ${callee}(null, ${args});
`);

const condition = Array.from(params)
.map(name => x`${name} !== ${block.renderer.reference(name.name)}`)
const condition = Array.from(args)
.map(name => x`${name} !== ${block.renderer.reference(alias_map.get(name.name) || name.name)}`)
.reduce((lhs, rhs) => x`${lhs} || ${rhs}`);

// we push unassign and unshift assign so that references are
Expand All @@ -62,7 +76,7 @@ export default function bind_this(component: Component, block: Block, binding: B
block.chunks.update.push(b`
if (${condition}) {
${unassign}();
${args.map(a => b`${a} = ${block.renderer.reference(a.name)}`)};
${args.map(a => b`${a} = ${block.renderer.reference(alias_map.get(a.name) || a.name)}`)};
${assign}();
}`
);
Expand Down
19 changes: 19 additions & 0 deletions src/compiler/compile/utils/compare_node.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { Node, Literal, Identifier, MemberExpression } from "estree";

export function compare_node(a: Node | void, b: Node | void) {
if (a === b) return true;
if (!a || !b) return false;
if (a.type !== b.type) return false;
switch (a.type) {
case "Identifier":
return a.name === (b as Identifier).name;
case "MemberExpression":
return (
compare_node(a.object, (b as MemberExpression).object) &&
compare_node(a.property, (b as MemberExpression).property) &&
a.computed === (b as MemberExpression).computed
);
case 'Literal':
return a.value === (b as Literal).value;
}
}
3 changes: 2 additions & 1 deletion test/js/samples/component-static-array/expected.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@ import {
} from "svelte/internal";

function create_fragment(ctx) {
let nested;
let current;
const nested = new /*Nested*/ ctx[0]({ props: { foo: [1, 2, 3] } });
nested = new /*Nested*/ ctx[0]({ props: { foo: [1, 2, 3] } });

return {
c() {
Expand Down
3 changes: 2 additions & 1 deletion test/js/samples/component-static-immutable/expected.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@ import {
} from "svelte/internal";

function create_fragment(ctx) {
let nested;
let current;
const nested = new /*Nested*/ ctx[0]({ props: { foo: "bar" } });
nested = new /*Nested*/ ctx[0]({ props: { foo: "bar" } });

return {
c() {
Expand Down
3 changes: 2 additions & 1 deletion test/js/samples/component-static-immutable2/expected.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@ import {
} from "svelte/internal";

function create_fragment(ctx) {
let nested;
let current;
const nested = new /*Nested*/ ctx[0]({ props: { foo: "bar" } });
nested = new /*Nested*/ ctx[0]({ props: { foo: "bar" } });

return {
c() {
Expand Down
6 changes: 4 additions & 2 deletions test/js/samples/component-static-var/expected.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,16 @@ import Foo from "./Foo.svelte";
import Bar from "./Bar.svelte";

function create_fragment(ctx) {
let foo;
let t0;
let bar;
let t1;
let input;
let current;
let mounted;
let dispose;
const foo = new Foo({ props: { x: y } });
const bar = new Bar({ props: { x: /*z*/ ctx[0] } });
foo = new Foo({ props: { x: y } });
bar = new Bar({ props: { x: /*z*/ ctx[0] } });

return {
c() {
Expand Down
3 changes: 2 additions & 1 deletion test/js/samples/component-static/expected.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@ import {
} from "svelte/internal";

function create_fragment(ctx) {
let nested;
let current;
const nested = new /*Nested*/ ctx[0]({ props: { foo: "bar" } });
nested = new /*Nested*/ ctx[0]({ props: { foo: "bar" } });

return {
c() {
Expand Down
3 changes: 2 additions & 1 deletion test/js/samples/dynamic-import/expected.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@ import {
import LazyLoad from "./LazyLoad.svelte";

function create_fragment(ctx) {
let lazyload;
let current;
const lazyload = new LazyLoad({ props: { load: func } });
lazyload = new LazyLoad({ props: { load: func } });

return {
c() {
Expand Down
6 changes: 4 additions & 2 deletions test/js/samples/non-imported-component/expected.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,12 @@ import {
import Imported from "Imported.svelte";

function create_fragment(ctx) {
let imported;
let t;
let nonimported;
let current;
const imported = new Imported({});
const nonimported = new NonImported({});
imported = new Imported({});
nonimported = new NonImported({});

return {
c() {
Expand Down

0 comments on commit 7dfd9e9

Please sign in to comment.