Skip to content

Commit 03afc7a

Browse files
Rich-HarrisRich-Harris
authored
Merge pull request #3876 from tanhauhau/tanhauhau/scoped-css-with-undefined-class
fix undefined class with scoped-css
2 parents 4ffa6fc + b22abc7 commit 03afc7a

File tree

5 files changed

+127
-92
lines changed

5 files changed

+127
-92
lines changed

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

+121-91
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import { b, x } from 'code-red';
77
import Expression from '../../../nodes/shared/Expression';
88
import Text from '../../../nodes/Text';
99
import { changed } from '../shared/changed';
10-
import { Literal } from 'estree';
1110

1211
export default class AttributeWrapper {
1312
node: Attribute;
@@ -72,85 +71,69 @@ export default class AttributeWrapper {
7271
const is_legacy_input_type = element.renderer.component.compile_options.legacy && name === 'type' && this.parent.node.name === 'input';
7372

7473
const dependencies = this.node.get_dependencies();
75-
if (dependencies.length > 0) {
76-
let value;
77-
78-
// TODO some of this code is repeated in Tag.ts — would be good to
79-
// DRY it out if that's possible without introducing crazy indirection
80-
if (this.node.chunks.length === 1) {
81-
// single {tag} — may be a non-string
82-
value = (this.node.chunks[0] as Expression).manipulate(block);
83-
} else {
84-
value = this.node.name === 'class'
85-
? this.get_class_name_text()
86-
: this.render_chunks().reduce((lhs, rhs) => x`${lhs} + ${rhs}`);
87-
88-
// '{foo} {bar}' — treat as string concatenation
89-
if (this.node.chunks[0].type !== 'Text') {
90-
value = x`"" + ${value}`;
91-
}
92-
}
74+
const value = this.get_value(block);
9375

94-
const is_select_value_attribute =
95-
name === 'value' && element.node.name === 'select';
76+
const is_select_value_attribute =
77+
name === 'value' && element.node.name === 'select';
9678

97-
const should_cache = is_select_value_attribute; // TODO is this necessary?
79+
const should_cache = is_select_value_attribute; // TODO is this necessary?
9880

99-
const last = should_cache && block.get_unique_name(
100-
`${element.var.name}_${name.replace(/[^a-zA-Z_$]/g, '_')}_value`
101-
);
81+
const last = should_cache && block.get_unique_name(
82+
`${element.var.name}_${name.replace(/[^a-zA-Z_$]/g, '_')}_value`
83+
);
10284

103-
if (should_cache) block.add_variable(last);
104-
105-
let updater;
106-
const init = should_cache ? x`${last} = ${value}` : value;
107-
108-
if (is_legacy_input_type) {
109-
block.chunks.hydrate.push(
110-
b`@set_input_type(${element.var}, ${init});`
111-
);
112-
updater = b`@set_input_type(${element.var}, ${should_cache ? last : value});`;
113-
} else if (is_select_value_attribute) {
114-
// annoying special case
115-
const is_multiple_select = element.node.get_static_attribute_value('multiple');
116-
const i = block.get_unique_name('i');
117-
const option = block.get_unique_name('option');
118-
119-
const if_statement = is_multiple_select
120-
? b`
121-
${option}.selected = ~${last}.indexOf(${option}.__value);`
122-
: b`
123-
if (${option}.__value === ${last}) {
124-
${option}.selected = true;
125-
${{ type: 'BreakStatement' }};
126-
}`; // TODO the BreakStatement is gross, but it's unsyntactic otherwise...
127-
128-
updater = b`
129-
for (var ${i} = 0; ${i} < ${element.var}.options.length; ${i} += 1) {
130-
var ${option} = ${element.var}.options[${i}];
131-
132-
${if_statement}
133-
}
134-
`;
135-
136-
block.chunks.mount.push(b`
137-
${last} = ${value};
138-
${updater}
139-
`);
140-
} else if (property_name) {
141-
block.chunks.hydrate.push(
142-
b`${element.var}.${property_name} = ${init};`
143-
);
144-
updater = block.renderer.options.dev
145-
? b`@prop_dev(${element.var}, "${property_name}", ${should_cache ? last : value});`
146-
: b`${element.var}.${property_name} = ${should_cache ? last : value};`;
147-
} else {
148-
block.chunks.hydrate.push(
149-
b`${method}(${element.var}, "${name}", ${init});`
150-
);
151-
updater = b`${method}(${element.var}, "${name}", ${should_cache ? last : value});`;
152-
}
85+
if (should_cache) block.add_variable(last);
86+
87+
let updater;
88+
const init = should_cache ? x`${last} = ${value}` : value;
89+
90+
if (is_legacy_input_type) {
91+
block.chunks.hydrate.push(
92+
b`@set_input_type(${element.var}, ${init});`
93+
);
94+
updater = b`@set_input_type(${element.var}, ${should_cache ? last : value});`;
95+
} else if (is_select_value_attribute) {
96+
// annoying special case
97+
const is_multiple_select = element.node.get_static_attribute_value('multiple');
98+
const i = block.get_unique_name('i');
99+
const option = block.get_unique_name('option');
100+
101+
const if_statement = is_multiple_select
102+
? b`
103+
${option}.selected = ~${last}.indexOf(${option}.__value);`
104+
: b`
105+
if (${option}.__value === ${last}) {
106+
${option}.selected = true;
107+
${{ type: 'BreakStatement' }};
108+
}`; // TODO the BreakStatement is gross, but it's unsyntactic otherwise...
109+
110+
updater = b`
111+
for (var ${i} = 0; ${i} < ${element.var}.options.length; ${i} += 1) {
112+
var ${option} = ${element.var}.options[${i}];
113+
114+
${if_statement}
115+
}
116+
`;
117+
118+
block.chunks.mount.push(b`
119+
${last} = ${value};
120+
${updater}
121+
`);
122+
} else if (property_name) {
123+
block.chunks.hydrate.push(
124+
b`${element.var}.${property_name} = ${init};`
125+
);
126+
updater = block.renderer.options.dev
127+
? b`@prop_dev(${element.var}, "${property_name}", ${should_cache ? last : value});`
128+
: b`${element.var}.${property_name} = ${should_cache ? last : value};`;
129+
} else {
130+
block.chunks.hydrate.push(
131+
b`${method}(${element.var}, "${name}", ${init});`
132+
);
133+
updater = b`${method}(${element.var}, "${name}", ${should_cache ? last : value});`;
134+
}
153135

136+
if (dependencies.length > 0) {
154137
let condition = changed(dependencies);
155138

156139
if (should_cache) {
@@ -165,23 +148,11 @@ export default class AttributeWrapper {
165148
if (${condition}) {
166149
${updater}
167150
}`);
168-
} else {
169-
const value = this.node.get_value(block);
170-
171-
const statement = (
172-
is_legacy_input_type
173-
? b`@set_input_type(${element.var}, ${value});`
174-
: property_name
175-
? b`${element.var}.${property_name} = ${value};`
176-
: b`${method}(${element.var}, "${name}", ${value.type === 'Literal' && (value as Literal).value === true ? x`""` : value});`
177-
);
178-
179-
block.chunks.hydrate.push(statement);
151+
}
180152

181-
// special case – autofocus. has to be handled in a bit of a weird way
182-
if (this.node.is_true && name === 'autofocus') {
183-
block.autofocus = element.var;
184-
}
153+
// special case – autofocus. has to be handled in a bit of a weird way
154+
if (this.node.is_true && name === 'autofocus') {
155+
block.autofocus = element.var;
185156
}
186157

187158
if (is_indirectly_bound_value) {
@@ -199,6 +170,36 @@ export default class AttributeWrapper {
199170
return metadata;
200171
}
201172

173+
get_value(block) {
174+
if (this.node.is_true) {
175+
const metadata = this.get_metadata();
176+
if (metadata && boolean_attribute.has(metadata.property_name.toLowerCase())) {
177+
return x`true`;
178+
}
179+
return x`""`;
180+
}
181+
if (this.node.chunks.length === 0) return x`""`;
182+
183+
// TODO some of this code is repeated in Tag.ts — would be good to
184+
// DRY it out if that's possible without introducing crazy indirection
185+
if (this.node.chunks.length === 1) {
186+
return this.node.chunks[0].type === 'Text'
187+
? string_literal((this.node.chunks[0] as Text).data)
188+
: (this.node.chunks[0] as Expression).manipulate(block);
189+
}
190+
191+
let value = this.node.name === 'class'
192+
? this.get_class_name_text()
193+
: this.render_chunks().reduce((lhs, rhs) => x`${lhs} + ${rhs}`);
194+
195+
// '{foo} {bar}' — treat as string concatenation
196+
if (this.node.chunks[0].type !== 'Text') {
197+
value = x`"" + ${value}`;
198+
}
199+
200+
return value;
201+
}
202+
202203
get_class_name_text() {
203204
const scoped_css = this.node.chunks.some((chunk: Text) => chunk.synthetic);
204205
const rendered = this.render_chunks();
@@ -292,3 +293,32 @@ Object.keys(attribute_lookup).forEach(name => {
292293
const metadata = attribute_lookup[name];
293294
if (!metadata.property_name) metadata.property_name = name;
294295
});
296+
297+
// source: https://html.spec.whatwg.org/multipage/indices.html
298+
const boolean_attribute = new Set([
299+
'allowfullscreen',
300+
'allowpaymentrequest',
301+
'async',
302+
'autofocus',
303+
'autoplay',
304+
'checked',
305+
'controls',
306+
'default',
307+
'defer',
308+
'disabled',
309+
'formnovalidate',
310+
'hidden',
311+
'ismap',
312+
'itemscope',
313+
'loop',
314+
'multiple',
315+
'muted',
316+
'nomodule',
317+
'novalidate',
318+
'open',
319+
'playsinline',
320+
'readonly',
321+
'required',
322+
'reversed',
323+
'selected'
324+
]);

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -617,7 +617,7 @@ export default class ElementWrapper extends Wrapper {
617617
const snippet = x`{ ${
618618
(metadata && metadata.property_name) ||
619619
fix_attribute_casing(attr.node.name)
620-
}: ${attr.node.get_value(block)} }`;
620+
}: ${attr.get_value(block)} }`;
621621
initial_props.push(snippet);
622622

623623
updates.push(condition ? x`${condition} && ${snippet}` : snippet);

‎test/css/samples/undefined-with-scope/expected.css

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
p.svelte-xyz{color:red}

‎test/css/samples/undefined-with-scope/expected.html

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
<p class=" svelte-xyz">Foo</p>

‎test/css/samples/undefined-with-scope/input.svelte

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
<style>p { color: red; }</style>
2+
3+
<p class={undefined}>Foo</p>

0 commit comments

Comments
 (0)