Skip to content

Commit

Permalink
fix binding for each block local variable (sveltejs#4861)
Browse files Browse the repository at this point in the history
  • Loading branch information
tanhauhau authored and taylorzane committed Dec 17, 2020
1 parent e054327 commit 799c9b3
Show file tree
Hide file tree
Showing 15 changed files with 437 additions and 172 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
# Svelte changelog

## Unreleased

* Fix `bind:this` to the value of an `{#each}` block ([#4517](https://github.com/sveltejs/svelte/issues/4517))
* Fix binding to contextual `{#each}` values that shadow outer names ([#4757](https://github.com/sveltejs/svelte/issues/4757))

## 3.23.0

* Update `<select>` with `bind:value` when the available `<option>`s change ([#1764](https://github.com/sveltejs/svelte/issues/1764))
Expand Down
37 changes: 19 additions & 18 deletions src/compiler/compile/render_dom/wrappers/Element/Binding.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import { b, x } from 'code-red';
import Binding from '../../../nodes/Binding';
import ElementWrapper from '../Element';
import InlineComponentWrapper from '../InlineComponent';
import get_object from '../../../utils/get_object';
import replace_object from '../../../utils/replace_object';
import Block from '../../Block';
import Renderer from '../../Renderer';
import flatten_reference from '../../../utils/flatten_reference';
Expand All @@ -10,20 +12,20 @@ import { Node, Identifier } from 'estree';

export default class BindingWrapper {
node: Binding;
parent: ElementWrapper;
parent: ElementWrapper | InlineComponentWrapper;

object: string;
handler: {
uses_context: boolean;
mutation: (Node | Node[]);
contextual_dependencies: Set<string>;
snippet?: Node;
lhs?: Node;
};
snippet: Node;
is_readonly: boolean;
needs_lock: boolean;

constructor(block: Block, node: Binding, parent: ElementWrapper) {
constructor(block: Block, node: Binding, parent: ElementWrapper | InlineComponentWrapper) {
this.node = node;
this.parent = parent;

Expand All @@ -33,7 +35,7 @@ export default class BindingWrapper {

// TODO does this also apply to e.g. `<input type='checkbox' bind:group='foo'>`?
if (parent.node.name === 'select') {
parent.select_binding_dependencies = dependencies;
(parent as ElementWrapper).select_binding_dependencies = dependencies;
dependencies.forEach((prop: string) => {
parent.renderer.component.indirect_dependencies.set(prop, new Set());
});
Expand Down Expand Up @@ -207,7 +209,7 @@ export default class BindingWrapper {
}

function get_dom_updater(
element: ElementWrapper,
element: ElementWrapper | InlineComponentWrapper,
binding: BindingWrapper
) {
const { node } = element;
Expand Down Expand Up @@ -270,21 +272,17 @@ function get_event_handler(
contextual_dependencies: Set<string>;
lhs?: Node;
} {
const value = get_value_from_dom(renderer, binding.parent, binding);
const contextual_dependencies = new Set(binding.node.expression.contextual_dependencies);
const contextual_dependencies = new Set<string>(binding.node.expression.contextual_dependencies);

const context = block.bindings.get(name);
let set_store;

if (context) {
const { object, property, modifier, store } = context;

if (lhs.type === 'Identifier') {
lhs = modifier(x`${object}[${property}]`);

contextual_dependencies.add(object.name);
contextual_dependencies.add(property.name);
}
const { object, property, store, snippet } = context;
lhs = replace_object(lhs, snippet);
contextual_dependencies.add(object.name);
contextual_dependencies.add(property.name);
contextual_dependencies.delete(name);

if (store) {
set_store = b`${store}.set(${`$${store}`});`;
Expand All @@ -297,6 +295,8 @@ function get_event_handler(
}
}

const value = get_value_from_dom(renderer, binding.parent, binding);

const mutation = b`
${lhs} = ${value};
${set_store}
Expand All @@ -305,20 +305,21 @@ function get_event_handler(
return {
uses_context: binding.node.is_contextual || binding.node.expression.uses_context, // TODO this is messy
mutation,
contextual_dependencies
contextual_dependencies,
lhs,
};
}

function get_value_from_dom(
renderer: Renderer,
element: ElementWrapper,
element: ElementWrapper | InlineComponentWrapper,
binding: BindingWrapper
) {
const { node } = element;
const { name } = binding.node;

if (name === 'this') {
return x`$$node`;
return x`$$value`;
}

// <select bind:value='selected>
Expand Down
Loading

0 comments on commit 799c9b3

Please sign in to comment.