Skip to content

Commit 4021063

Browse files
tanhauhautanhauhau
authored
spread condition for input element (#5004)
1 parent d6392ad commit 4021063

9 files changed

Lines changed: 240 additions & 82 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
* Support nullish coalescing (`??`) and optional chaining (`?.`) operators ([#1972](https://github.com/sveltejs/svelte/issues/1972))
66
* Support `import.meta` ([#4379](https://github.com/sveltejs/svelte/issues/4379))
7+
* Fix only setting `<input>` values when they're changed when there are spread attributes ([#4418](https://github.com/sveltejs/svelte/issues/4418))
78
* Fix placement of `{@html}` when used at the root of a slot, at the root of a component, or in `<svelte:head>` ([#5012](https://github.com/sveltejs/svelte/issues/5012), [#5071](https://github.com/sveltejs/svelte/pull/5071))
89
* Fix certain handling of two-way bound `contenteditable` elements ([#5018](https://github.com/sveltejs/svelte/issues/5018))
910
* Fix handling of `import`ed value that is used as a store and is also mutated ([#5019](https://github.com/sveltejs/svelte/issues/5019))

‎src/compiler/compile/render_dom/wrappers/Element/Attribute.ts‎

Lines changed: 107 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,9 @@ import { b, x } from 'code-red';
77
import Expression from '../../../nodes/shared/Expression';
88
import Text from '../../../nodes/Text';
99
import handle_select_value_binding from './handle_select_value_binding';
10+
import { Identifier, Node } from 'estree';
1011

11-
export default class AttributeWrapper {
12+
export class BaseAttributeWrapper {
1213
node: Attribute;
1314
parent: ElementWrapper;
1415

@@ -21,7 +22,29 @@ export default class AttributeWrapper {
2122
parent.not_static_content();
2223

2324
block.add_dependencies(node.dependencies);
25+
}
26+
}
27+
28+
render(_block: Block) {}
29+
}
2430

31+
export default class AttributeWrapper extends BaseAttributeWrapper {
32+
node: Attribute;
33+
parent: ElementWrapper;
34+
metadata: any;
35+
name: string;
36+
property_name: string;
37+
is_indirectly_bound_value: boolean;
38+
is_src: boolean;
39+
is_select_value_attribute: boolean;
40+
is_input_value: boolean;
41+
should_cache: boolean;
42+
last: Identifier;
43+
44+
constructor(parent: ElementWrapper, block: Block, node: Attribute) {
45+
super(parent, block, node);
46+
47+
if (node.dependencies.size > 0) {
2548
// special case — <option value={foo}> — see below
2649
if (this.parent.node.name === 'option' && node.name === 'value') {
2750
let select: ElementWrapper = this.parent;
@@ -42,31 +65,22 @@ export default class AttributeWrapper {
4265
handle_select_value_binding(this, node.dependencies);
4366
}
4467
}
45-
}
4668

47-
is_indirectly_bound_value() {
48-
const element = this.parent;
49-
const name = fix_attribute_casing(this.node.name);
50-
return name === 'value' &&
51-
(element.node.name === 'option' || // TODO check it's actually bound
52-
(element.node.name === 'input' &&
53-
element.node.bindings.some(
54-
(binding) =>
55-
/checked|group/.test(binding.name)
56-
)));
69+
this.name = fix_attribute_casing(this.node.name);
70+
this.metadata = this.get_metadata();
71+
this.is_indirectly_bound_value = is_indirectly_bound_value(this);
72+
this.property_name = this.is_indirectly_bound_value
73+
? '__value'
74+
: this.metadata && this.metadata.property_name;
75+
this.is_src = this.name === 'src'; // TODO retire this exception in favour of https://github.com/sveltejs/svelte/issues/3750
76+
this.is_select_value_attribute = this.name === 'value' && this.parent.node.name === 'select';
77+
this.is_input_value = this.name === 'value' && this.parent.node.name === 'input';
78+
this.should_cache = should_cache(this);
5779
}
5880

5981
render(block: Block) {
6082
const element = this.parent;
61-
const name = fix_attribute_casing(this.node.name);
62-
63-
const metadata = this.get_metadata();
64-
65-
const is_indirectly_bound_value = this.is_indirectly_bound_value();
66-
67-
const property_name = is_indirectly_bound_value
68-
? '__value'
69-
: metadata && metadata.property_name;
83+
const { name, property_name, should_cache, is_indirectly_bound_value } = this;
7084

7185
// xlink is a special case... we could maybe extend this to generic
7286
// namespaced attributes but I'm not sure that's applicable in
@@ -82,29 +96,15 @@ export default class AttributeWrapper {
8296
const dependencies = this.get_dependencies();
8397
const value = this.get_value(block);
8498

85-
const is_src = this.node.name === 'src'; // TODO retire this exception in favour of https://github.com/sveltejs/svelte/issues/3750
86-
const is_select_value_attribute =
87-
name === 'value' && element.node.name === 'select';
88-
89-
const is_input_value = name === 'value' && element.node.name === 'input';
90-
91-
const should_cache = is_src || this.node.should_cache();
92-
93-
const last = should_cache && block.get_unique_name(
94-
`${element.var.name}_${name.replace(/[^a-zA-Z_$]/g, '_')}_value`
95-
);
96-
97-
if (should_cache) block.add_variable(last);
98-
9999
let updater;
100-
const init = should_cache ? x`${last} = ${value}` : value;
100+
const init = this.get_init(block, value);
101101

102102
if (is_legacy_input_type) {
103103
block.chunks.hydrate.push(
104104
b`@set_input_type(${element.var}, ${init});`
105105
);
106-
updater = b`@set_input_type(${element.var}, ${should_cache ? last : value});`;
107-
} else if (is_select_value_attribute) {
106+
updater = b`@set_input_type(${element.var}, ${should_cache ? this.last : value});`;
107+
} else if (this.is_select_value_attribute) {
108108
// annoying special case
109109
const is_multiple_select = element.node.get_static_attribute_value('multiple');
110110

@@ -117,45 +117,37 @@ export default class AttributeWrapper {
117117
block.chunks.mount.push(b`
118118
${updater}
119119
`);
120-
} else if (is_src) {
120+
} else if (this.is_src) {
121121
block.chunks.hydrate.push(
122-
b`if (${element.var}.src !== ${init}) ${method}(${element.var}, "${name}", ${last});`
122+
b`if (${element.var}.src !== ${init}) ${method}(${element.var}, "${name}", ${this.last});`
123123
);
124-
updater = b`${method}(${element.var}, "${name}", ${should_cache ? last : value});`;
124+
updater = b`${method}(${element.var}, "${name}", ${should_cache ? this.last : value});`;
125125
} else if (property_name) {
126126
block.chunks.hydrate.push(
127127
b`${element.var}.${property_name} = ${init};`
128128
);
129129
updater = block.renderer.options.dev
130-
? b`@prop_dev(${element.var}, "${property_name}", ${should_cache ? last : value});`
131-
: b`${element.var}.${property_name} = ${should_cache ? last : value};`;
130+
? b`@prop_dev(${element.var}, "${property_name}", ${should_cache ? this.last : value});`
131+
: b`${element.var}.${property_name} = ${should_cache ? this.last : value};`;
132132
} else {
133133
block.chunks.hydrate.push(
134134
b`${method}(${element.var}, "${name}", ${init});`
135135
);
136-
updater = b`${method}(${element.var}, "${name}", ${should_cache ? last : value});`;
136+
updater = b`${method}(${element.var}, "${name}", ${should_cache ? this.last : value});`;
137137
}
138138

139-
if (dependencies.length > 0) {
140-
let condition = block.renderer.dirty(dependencies);
141-
142-
if (should_cache) {
143-
condition = is_src
144-
? x`${condition} && (${element.var}.src !== (${last} = ${value}))`
145-
: x`${condition} && (${last} !== (${last} = ${value}))`;
146-
}
147-
148-
if (is_input_value) {
149-
const type = element.node.get_static_attribute_value('type');
139+
if (is_indirectly_bound_value) {
140+
const update_value = b`${element.var}.value = ${element.var}.__value;`;
141+
block.chunks.hydrate.push(update_value);
150142

151-
if (type === null || type === "" || type === "text" || type === "email" || type === "password") {
152-
condition = x`${condition} && ${element.var}.${property_name} !== ${should_cache ? last : value}`;
153-
}
154-
}
143+
updater = b`
144+
${updater}
145+
${update_value};
146+
`;
147+
}
155148

156-
if (block.has_outros) {
157-
condition = x`!#current || ${condition}`;
158-
}
149+
if (dependencies.length > 0) {
150+
const condition = this.get_dom_update_conditions(block, block.renderer.dirty(dependencies));
159151

160152
block.chunks.update.push(b`
161153
if (${condition}) {
@@ -167,13 +159,44 @@ export default class AttributeWrapper {
167159
if (this.node.is_true && name === 'autofocus') {
168160
block.autofocus = element.var;
169161
}
162+
}
170163

171-
if (is_indirectly_bound_value) {
172-
const update_value = b`${element.var}.value = ${element.var}.__value;`;
164+
get_init(block: Block, value) {
165+
this.last = this.should_cache && block.get_unique_name(
166+
`${this.parent.var.name}_${this.name.replace(/[^a-zA-Z_$]/g, '_')}_value`
167+
);
173168

174-
block.chunks.hydrate.push(update_value);
175-
if (dependencies.length > 0) block.chunks.update.push(update_value);
169+
if (this.should_cache) block.add_variable(this.last);
170+
171+
return this.should_cache ? x`${this.last} = ${value}` : value;
172+
}
173+
174+
get_dom_update_conditions(block: Block, dependency_condition: Node) {
175+
const { property_name, should_cache, last } = this;
176+
const element = this.parent;
177+
const value = this.get_value(block);
178+
179+
let condition = dependency_condition;
180+
181+
if (should_cache) {
182+
condition = this.is_src
183+
? x`${condition} && (${element.var}.src !== (${last} = ${value}))`
184+
: x`${condition} && (${last} !== (${last} = ${value}))`;
185+
}
186+
187+
if (this.is_input_value) {
188+
const type = element.node.get_static_attribute_value('type');
189+
190+
if (type === null || type === "" || type === "text" || type === "email" || type === "password") {
191+
condition = x`${condition} && ${element.var}.${property_name} !== ${should_cache ? last : value}`;
192+
}
193+
}
194+
195+
if (block.has_outros) {
196+
condition = x`!#current || ${condition}`;
176197
}
198+
199+
return condition;
177200
}
178201

179202
get_dependencies() {
@@ -194,15 +217,14 @@ export default class AttributeWrapper {
194217

195218
get_metadata() {
196219
if (this.parent.node.namespace) return null;
197-
const metadata = attribute_lookup[fix_attribute_casing(this.node.name)];
220+
const metadata = attribute_lookup[this.name];
198221
if (metadata && metadata.applies_to && !metadata.applies_to.includes(this.parent.node.name)) return null;
199222
return metadata;
200223
}
201224

202225
get_value(block) {
203226
if (this.node.is_true) {
204-
const metadata = this.get_metadata();
205-
if (metadata && boolean_attribute.has(metadata.property_name.toLowerCase())) {
227+
if (this.metadata && boolean_attribute.has(this.metadata.property_name.toLowerCase())) {
206228
return x`true`;
207229
}
208230
return x`""`;
@@ -350,4 +372,19 @@ const boolean_attribute = new Set([
350372
'required',
351373
'reversed',
352374
'selected'
353-
]);
375+
]);
376+
377+
function should_cache(attribute: AttributeWrapper) {
378+
return attribute.is_src || attribute.node.should_cache();
379+
}
380+
381+
function is_indirectly_bound_value(attribute: AttributeWrapper) {
382+
const element = attribute.parent;
383+
return attribute.name === 'value' &&
384+
(element.node.name === 'option' || // TODO check it's actually bound
385+
(element.node.name === 'input' &&
386+
element.node.bindings.some(
387+
(binding) =>
388+
/checked|group/.test(binding.name)
389+
)));
390+
}

‎src/compiler/compile/render_dom/wrappers/Element/SpreadAttribute.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
import { BaseAttributeWrapper } from "./Attribute";
2+
3+
export default class SpreadAttributeWrapper extends BaseAttributeWrapper {}

‎src/compiler/compile/render_dom/wrappers/Element/index.ts‎

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { b, x, p } from 'code-red';
1111
import { namespaces } from '../../../../utils/namespaces';
1212
import AttributeWrapper from './Attribute';
1313
import StyleAttributeWrapper from './StyleAttribute';
14+
import SpreadAttributeWrapper from './SpreadAttribute';
1415
import { dimensions } from '../../../../utils/patterns';
1516
import Binding from './Binding';
1617
import InlineComponentWrapper from '../InlineComponent';
@@ -137,7 +138,7 @@ const events = [
137138
export default class ElementWrapper extends Wrapper {
138139
node: Element;
139140
fragment: FragmentWrapper;
140-
attributes: AttributeWrapper[];
141+
attributes: Array<AttributeWrapper | StyleAttributeWrapper | SpreadAttributeWrapper>;
141142
bindings: Binding[];
142143
event_handlers: EventHandler[];
143144
class_dependencies: string[];
@@ -221,6 +222,9 @@ export default class ElementWrapper extends Wrapper {
221222
if (attribute.name === 'style') {
222223
return new StyleAttributeWrapper(this, block, attribute);
223224
}
225+
if (attribute.type === 'Spread') {
226+
return new SpreadAttributeWrapper(this, block, attribute);
227+
}
224228
return new AttributeWrapper(this, block, attribute);
225229
});
226230

@@ -418,7 +422,7 @@ export default class ElementWrapper extends Wrapper {
418422
return x`@_document.createElementNS("${namespace}", "${name}")`;
419423
}
420424

421-
const is = this.attributes.find(attr => attr.node.name === 'is');
425+
const is: AttributeWrapper = this.attributes.find(attr => attr.node.name === 'is') as any;
422426
if (is) {
423427
return x`@element_is("${name}", ${is.render_chunks(block).reduce((lhs, rhs) => x`${lhs} + ${rhs}`)})`;
424428
}
@@ -664,25 +668,24 @@ export default class ElementWrapper extends Wrapper {
664668

665669
this.attributes
666670
.forEach(attr => {
667-
const condition = attr.node.dependencies.size > 0
668-
? block.renderer.dirty(Array.from(attr.node.dependencies))
671+
const dependencies = attr.node.get_dependencies();
672+
673+
const condition = dependencies.length > 0
674+
? block.renderer.dirty(dependencies)
669675
: null;
670676

671-
if (attr.node.is_spread) {
677+
if (attr instanceof SpreadAttributeWrapper) {
672678
const snippet = attr.node.expression.manipulate(block);
673679

674680
initial_props.push(snippet);
675681

676682
updates.push(condition ? x`${condition} && ${snippet}` : snippet);
677683
} else {
678-
const metadata = attr.get_metadata();
679-
const name = attr.is_indirectly_bound_value()
680-
? '__value'
681-
: (metadata && metadata.property_name) || fix_attribute_casing(attr.node.name);
682-
const snippet = x`{ ${name}: ${attr.get_value(block)} }`;
683-
initial_props.push(snippet);
684+
const name = attr.property_name || attr.name;
685+
initial_props.push(x`{ ${name}: ${attr.get_init(block, attr.get_value(block))} }`);
686+
const snippet = x`{ ${name}: ${attr.should_cache ? attr.last : attr.get_value(block)} }`;
684687

685-
updates.push(condition ? x`${condition} && ${snippet}` : snippet);
688+
updates.push(condition ? x`${attr.get_dom_update_conditions(block, condition)} && ${snippet}` : snippet);
686689
}
687690
});
688691

‎test/runtime/samples/spread-element-input-value/InputOne.svelte‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<script>
2+
import { omit } from './utils.js';
3+
4+
export let value;
5+
6+
function onInput(e) {
7+
value = e.target.value;
8+
}
9+
10+
$: props = omit($$props, 'value');
11+
</script>
12+
13+
<input
14+
type="text"
15+
{...props}
16+
on:input={onInput}
17+
{value}
18+
/>

‎test/runtime/samples/spread-element-input-value/InputTwo.svelte‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<script>
2+
import { omit } from './utils.js';
3+
4+
export let value;
5+
6+
function onInput(e) {
7+
value = e.target.value;
8+
}
9+
10+
$: props = omit($$props, 'value', 'minlength');
11+
</script>
12+
13+
<input
14+
type="text"
15+
minlength="10"
16+
value={value}
17+
{...props}
18+
on:input={onInput}
19+
/>

0 commit comments

Comments
 (0)