diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0088e4b0b395..d6fcc626c8c3 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,7 @@
* Support nullish coalescing (`??`) and optional chaining (`?.`) operators ([#1972](https://github.com/sveltejs/svelte/issues/1972))
* Support `import.meta` ([#4379](https://github.com/sveltejs/svelte/issues/4379))
+* Fix only setting ` ` values when they're changed when there are spread attributes ([#4418](https://github.com/sveltejs/svelte/issues/4418))
* Fix placement of `{@html}` when used at the root of a slot, at the root of a component, or in `` ([#5012](https://github.com/sveltejs/svelte/issues/5012), [#5071](https://github.com/sveltejs/svelte/pull/5071))
* Fix certain handling of two-way bound `contenteditable` elements ([#5018](https://github.com/sveltejs/svelte/issues/5018))
* Fix handling of `import`ed value that is used as a store and is also mutated ([#5019](https://github.com/sveltejs/svelte/issues/5019))
diff --git a/src/compiler/compile/render_dom/wrappers/Element/Attribute.ts b/src/compiler/compile/render_dom/wrappers/Element/Attribute.ts
index 86b67ca47e37..ec281648b895 100644
--- a/src/compiler/compile/render_dom/wrappers/Element/Attribute.ts
+++ b/src/compiler/compile/render_dom/wrappers/Element/Attribute.ts
@@ -7,8 +7,9 @@ import { b, x } from 'code-red';
import Expression from '../../../nodes/shared/Expression';
import Text from '../../../nodes/Text';
import handle_select_value_binding from './handle_select_value_binding';
+import { Identifier, Node } from 'estree';
-export default class AttributeWrapper {
+export class BaseAttributeWrapper {
node: Attribute;
parent: ElementWrapper;
@@ -21,7 +22,29 @@ export default class AttributeWrapper {
parent.not_static_content();
block.add_dependencies(node.dependencies);
+ }
+ }
+
+ render(_block: Block) {}
+}
+export default class AttributeWrapper extends BaseAttributeWrapper {
+ node: Attribute;
+ parent: ElementWrapper;
+ metadata: any;
+ name: string;
+ property_name: string;
+ is_indirectly_bound_value: boolean;
+ is_src: boolean;
+ is_select_value_attribute: boolean;
+ is_input_value: boolean;
+ should_cache: boolean;
+ last: Identifier;
+
+ constructor(parent: ElementWrapper, block: Block, node: Attribute) {
+ super(parent, block, node);
+
+ if (node.dependencies.size > 0) {
// special case — — see below
if (this.parent.node.name === 'option' && node.name === 'value') {
let select: ElementWrapper = this.parent;
@@ -42,31 +65,22 @@ export default class AttributeWrapper {
handle_select_value_binding(this, node.dependencies);
}
}
- }
- is_indirectly_bound_value() {
- const element = this.parent;
- const name = fix_attribute_casing(this.node.name);
- return name === 'value' &&
- (element.node.name === 'option' || // TODO check it's actually bound
- (element.node.name === 'input' &&
- element.node.bindings.some(
- (binding) =>
- /checked|group/.test(binding.name)
- )));
+ this.name = fix_attribute_casing(this.node.name);
+ this.metadata = this.get_metadata();
+ this.is_indirectly_bound_value = is_indirectly_bound_value(this);
+ this.property_name = this.is_indirectly_bound_value
+ ? '__value'
+ : this.metadata && this.metadata.property_name;
+ this.is_src = this.name === 'src'; // TODO retire this exception in favour of https://github.com/sveltejs/svelte/issues/3750
+ this.is_select_value_attribute = this.name === 'value' && this.parent.node.name === 'select';
+ this.is_input_value = this.name === 'value' && this.parent.node.name === 'input';
+ this.should_cache = should_cache(this);
}
render(block: Block) {
const element = this.parent;
- const name = fix_attribute_casing(this.node.name);
-
- const metadata = this.get_metadata();
-
- const is_indirectly_bound_value = this.is_indirectly_bound_value();
-
- const property_name = is_indirectly_bound_value
- ? '__value'
- : metadata && metadata.property_name;
+ const { name, property_name, should_cache, is_indirectly_bound_value } = this;
// xlink is a special case... we could maybe extend this to generic
// namespaced attributes but I'm not sure that's applicable in
@@ -82,29 +96,15 @@ export default class AttributeWrapper {
const dependencies = this.get_dependencies();
const value = this.get_value(block);
- const is_src = this.node.name === 'src'; // TODO retire this exception in favour of https://github.com/sveltejs/svelte/issues/3750
- const is_select_value_attribute =
- name === 'value' && element.node.name === 'select';
-
- const is_input_value = name === 'value' && element.node.name === 'input';
-
- const should_cache = is_src || this.node.should_cache();
-
- const last = should_cache && block.get_unique_name(
- `${element.var.name}_${name.replace(/[^a-zA-Z_$]/g, '_')}_value`
- );
-
- if (should_cache) block.add_variable(last);
-
let updater;
- const init = should_cache ? x`${last} = ${value}` : value;
+ const init = this.get_init(block, value);
if (is_legacy_input_type) {
block.chunks.hydrate.push(
b`@set_input_type(${element.var}, ${init});`
);
- updater = b`@set_input_type(${element.var}, ${should_cache ? last : value});`;
- } else if (is_select_value_attribute) {
+ updater = b`@set_input_type(${element.var}, ${should_cache ? this.last : value});`;
+ } else if (this.is_select_value_attribute) {
// annoying special case
const is_multiple_select = element.node.get_static_attribute_value('multiple');
@@ -117,45 +117,37 @@ export default class AttributeWrapper {
block.chunks.mount.push(b`
${updater}
`);
- } else if (is_src) {
+ } else if (this.is_src) {
block.chunks.hydrate.push(
- b`if (${element.var}.src !== ${init}) ${method}(${element.var}, "${name}", ${last});`
+ b`if (${element.var}.src !== ${init}) ${method}(${element.var}, "${name}", ${this.last});`
);
- updater = b`${method}(${element.var}, "${name}", ${should_cache ? last : value});`;
+ updater = b`${method}(${element.var}, "${name}", ${should_cache ? this.last : value});`;
} else if (property_name) {
block.chunks.hydrate.push(
b`${element.var}.${property_name} = ${init};`
);
updater = block.renderer.options.dev
- ? b`@prop_dev(${element.var}, "${property_name}", ${should_cache ? last : value});`
- : b`${element.var}.${property_name} = ${should_cache ? last : value};`;
+ ? b`@prop_dev(${element.var}, "${property_name}", ${should_cache ? this.last : value});`
+ : b`${element.var}.${property_name} = ${should_cache ? this.last : value};`;
} else {
block.chunks.hydrate.push(
b`${method}(${element.var}, "${name}", ${init});`
);
- updater = b`${method}(${element.var}, "${name}", ${should_cache ? last : value});`;
+ updater = b`${method}(${element.var}, "${name}", ${should_cache ? this.last : value});`;
}
- if (dependencies.length > 0) {
- let condition = block.renderer.dirty(dependencies);
-
- if (should_cache) {
- condition = is_src
- ? x`${condition} && (${element.var}.src !== (${last} = ${value}))`
- : x`${condition} && (${last} !== (${last} = ${value}))`;
- }
-
- if (is_input_value) {
- const type = element.node.get_static_attribute_value('type');
+ if (is_indirectly_bound_value) {
+ const update_value = b`${element.var}.value = ${element.var}.__value;`;
+ block.chunks.hydrate.push(update_value);
- if (type === null || type === "" || type === "text" || type === "email" || type === "password") {
- condition = x`${condition} && ${element.var}.${property_name} !== ${should_cache ? last : value}`;
- }
- }
+ updater = b`
+ ${updater}
+ ${update_value};
+ `;
+ }
- if (block.has_outros) {
- condition = x`!#current || ${condition}`;
- }
+ if (dependencies.length > 0) {
+ const condition = this.get_dom_update_conditions(block, block.renderer.dirty(dependencies));
block.chunks.update.push(b`
if (${condition}) {
@@ -167,13 +159,44 @@ export default class AttributeWrapper {
if (this.node.is_true && name === 'autofocus') {
block.autofocus = element.var;
}
+ }
- if (is_indirectly_bound_value) {
- const update_value = b`${element.var}.value = ${element.var}.__value;`;
+ get_init(block: Block, value) {
+ this.last = this.should_cache && block.get_unique_name(
+ `${this.parent.var.name}_${this.name.replace(/[^a-zA-Z_$]/g, '_')}_value`
+ );
- block.chunks.hydrate.push(update_value);
- if (dependencies.length > 0) block.chunks.update.push(update_value);
+ if (this.should_cache) block.add_variable(this.last);
+
+ return this.should_cache ? x`${this.last} = ${value}` : value;
+ }
+
+ get_dom_update_conditions(block: Block, dependency_condition: Node) {
+ const { property_name, should_cache, last } = this;
+ const element = this.parent;
+ const value = this.get_value(block);
+
+ let condition = dependency_condition;
+
+ if (should_cache) {
+ condition = this.is_src
+ ? x`${condition} && (${element.var}.src !== (${last} = ${value}))`
+ : x`${condition} && (${last} !== (${last} = ${value}))`;
+ }
+
+ if (this.is_input_value) {
+ const type = element.node.get_static_attribute_value('type');
+
+ if (type === null || type === "" || type === "text" || type === "email" || type === "password") {
+ condition = x`${condition} && ${element.var}.${property_name} !== ${should_cache ? last : value}`;
+ }
+ }
+
+ if (block.has_outros) {
+ condition = x`!#current || ${condition}`;
}
+
+ return condition;
}
get_dependencies() {
@@ -194,15 +217,14 @@ export default class AttributeWrapper {
get_metadata() {
if (this.parent.node.namespace) return null;
- const metadata = attribute_lookup[fix_attribute_casing(this.node.name)];
+ const metadata = attribute_lookup[this.name];
if (metadata && metadata.applies_to && !metadata.applies_to.includes(this.parent.node.name)) return null;
return metadata;
}
get_value(block) {
if (this.node.is_true) {
- const metadata = this.get_metadata();
- if (metadata && boolean_attribute.has(metadata.property_name.toLowerCase())) {
+ if (this.metadata && boolean_attribute.has(this.metadata.property_name.toLowerCase())) {
return x`true`;
}
return x`""`;
@@ -350,4 +372,19 @@ const boolean_attribute = new Set([
'required',
'reversed',
'selected'
-]);
\ No newline at end of file
+]);
+
+function should_cache(attribute: AttributeWrapper) {
+ return attribute.is_src || attribute.node.should_cache();
+}
+
+function is_indirectly_bound_value(attribute: AttributeWrapper) {
+ const element = attribute.parent;
+ return attribute.name === 'value' &&
+ (element.node.name === 'option' || // TODO check it's actually bound
+ (element.node.name === 'input' &&
+ element.node.bindings.some(
+ (binding) =>
+ /checked|group/.test(binding.name)
+ )));
+}
\ No newline at end of file
diff --git a/src/compiler/compile/render_dom/wrappers/Element/SpreadAttribute.ts b/src/compiler/compile/render_dom/wrappers/Element/SpreadAttribute.ts
new file mode 100644
index 000000000000..a47ddc892bd4
--- /dev/null
+++ b/src/compiler/compile/render_dom/wrappers/Element/SpreadAttribute.ts
@@ -0,0 +1,3 @@
+import { BaseAttributeWrapper } from "./Attribute";
+
+export default class SpreadAttributeWrapper extends BaseAttributeWrapper {}
diff --git a/src/compiler/compile/render_dom/wrappers/Element/index.ts b/src/compiler/compile/render_dom/wrappers/Element/index.ts
index 148f837ee6d7..f11ce630237a 100644
--- a/src/compiler/compile/render_dom/wrappers/Element/index.ts
+++ b/src/compiler/compile/render_dom/wrappers/Element/index.ts
@@ -11,6 +11,7 @@ import { b, x, p } from 'code-red';
import { namespaces } from '../../../../utils/namespaces';
import AttributeWrapper from './Attribute';
import StyleAttributeWrapper from './StyleAttribute';
+import SpreadAttributeWrapper from './SpreadAttribute';
import { dimensions } from '../../../../utils/patterns';
import Binding from './Binding';
import InlineComponentWrapper from '../InlineComponent';
@@ -137,7 +138,7 @@ const events = [
export default class ElementWrapper extends Wrapper {
node: Element;
fragment: FragmentWrapper;
- attributes: AttributeWrapper[];
+ attributes: Array;
bindings: Binding[];
event_handlers: EventHandler[];
class_dependencies: string[];
@@ -221,6 +222,9 @@ export default class ElementWrapper extends Wrapper {
if (attribute.name === 'style') {
return new StyleAttributeWrapper(this, block, attribute);
}
+ if (attribute.type === 'Spread') {
+ return new SpreadAttributeWrapper(this, block, attribute);
+ }
return new AttributeWrapper(this, block, attribute);
});
@@ -418,7 +422,7 @@ export default class ElementWrapper extends Wrapper {
return x`@_document.createElementNS("${namespace}", "${name}")`;
}
- const is = this.attributes.find(attr => attr.node.name === 'is');
+ const is: AttributeWrapper = this.attributes.find(attr => attr.node.name === 'is') as any;
if (is) {
return x`@element_is("${name}", ${is.render_chunks(block).reduce((lhs, rhs) => x`${lhs} + ${rhs}`)})`;
}
@@ -664,25 +668,24 @@ export default class ElementWrapper extends Wrapper {
this.attributes
.forEach(attr => {
- const condition = attr.node.dependencies.size > 0
- ? block.renderer.dirty(Array.from(attr.node.dependencies))
+ const dependencies = attr.node.get_dependencies();
+
+ const condition = dependencies.length > 0
+ ? block.renderer.dirty(dependencies)
: null;
- if (attr.node.is_spread) {
+ if (attr instanceof SpreadAttributeWrapper) {
const snippet = attr.node.expression.manipulate(block);
initial_props.push(snippet);
updates.push(condition ? x`${condition} && ${snippet}` : snippet);
} else {
- const metadata = attr.get_metadata();
- const name = attr.is_indirectly_bound_value()
- ? '__value'
- : (metadata && metadata.property_name) || fix_attribute_casing(attr.node.name);
- const snippet = x`{ ${name}: ${attr.get_value(block)} }`;
- initial_props.push(snippet);
+ const name = attr.property_name || attr.name;
+ initial_props.push(x`{ ${name}: ${attr.get_init(block, attr.get_value(block))} }`);
+ const snippet = x`{ ${name}: ${attr.should_cache ? attr.last : attr.get_value(block)} }`;
- updates.push(condition ? x`${condition} && ${snippet}` : snippet);
+ updates.push(condition ? x`${attr.get_dom_update_conditions(block, condition)} && ${snippet}` : snippet);
}
});
diff --git a/test/runtime/samples/spread-element-input-value/InputOne.svelte b/test/runtime/samples/spread-element-input-value/InputOne.svelte
new file mode 100644
index 000000000000..92ecb7b9a98a
--- /dev/null
+++ b/test/runtime/samples/spread-element-input-value/InputOne.svelte
@@ -0,0 +1,18 @@
+
+
+
diff --git a/test/runtime/samples/spread-element-input-value/InputTwo.svelte b/test/runtime/samples/spread-element-input-value/InputTwo.svelte
new file mode 100644
index 000000000000..33ec0622a46a
--- /dev/null
+++ b/test/runtime/samples/spread-element-input-value/InputTwo.svelte
@@ -0,0 +1,19 @@
+
+
+
diff --git a/test/runtime/samples/spread-element-input-value/_config.js b/test/runtime/samples/spread-element-input-value/_config.js
new file mode 100644
index 000000000000..a07de8241987
--- /dev/null
+++ b/test/runtime/samples/spread-element-input-value/_config.js
@@ -0,0 +1,62 @@
+export default {
+ async test({ assert, component, target, window }) {
+ const [input1, input2] = target.querySelectorAll("input");
+
+ // we are not able emulate user interaction in jsdom,
+ // therefore, jsdom could not validate minlength / maxlength
+
+ // we simulate user input with
+ // setting input.value + dispathEvent
+
+ // and we determine if svelte does not set the `input.value` again by
+ // spying on the setter of `input.value`
+
+ const spy1 = spyOnValueSetter(input1, input1.value);
+ const spy2 = spyOnValueSetter(input2, input2.value);
+
+ const event = new window.Event("input");
+
+ input1.value = '12345';
+ spy1.reset();
+ await input1.dispatchEvent(event);
+
+ assert.ok(!spy1.isSetCalled());
+
+ input2.value = '12345';
+ spy2.reset();
+ await input2.dispatchEvent(event);
+
+ assert.ok(!spy2.isSetCalled());
+
+ spy1.reset();
+ component.val1 = '56789';
+ assert.ok(spy1.isSetCalled());
+
+ spy2.reset();
+ component.val2 = '56789';
+ assert.ok(spy2.isSetCalled());
+ },
+};
+
+function spyOnValueSetter(input, initialValue) {
+ let value = initialValue;
+ let isSet = false;
+ Object.defineProperty(input, "value", {
+ get() {
+ return value;
+ },
+ set(_value) {
+ value = _value;
+ isSet = true;
+ },
+ });
+
+ return {
+ isSetCalled() {
+ return isSet;
+ },
+ reset() {
+ isSet = false;
+ },
+ };
+}
diff --git a/test/runtime/samples/spread-element-input-value/main.svelte b/test/runtime/samples/spread-element-input-value/main.svelte
new file mode 100644
index 000000000000..bb5f0e00bf48
--- /dev/null
+++ b/test/runtime/samples/spread-element-input-value/main.svelte
@@ -0,0 +1,9 @@
+
+
+
+
\ No newline at end of file
diff --git a/test/runtime/samples/spread-element-input-value/utils.js b/test/runtime/samples/spread-element-input-value/utils.js
new file mode 100644
index 000000000000..ee941bda5545
--- /dev/null
+++ b/test/runtime/samples/spread-element-input-value/utils.js
@@ -0,0 +1,6 @@
+export function omit(obj, ...keysToOmit) {
+ return Object.keys(obj).reduce((acc, key) => {
+ if (keysToOmit.indexOf(key) === -1) acc[key] = obj[key];
+ return acc;
+ }, {});
+}