msb-public
/
svelte
mirror of https://github.com/sveltejs/svelte
1
0
Code Issues Projects Releases Wiki Activity

Fix boolean attributes in presence of spread attributes (#3775)

Browse Source 
* add failing tests

* fix boolean attributes along with spreads (DOM mode)

* fix boolean attributes along with spreads (SSR mode)

* update changelog (#3764)

* fix removing attributes in spreads
pull/3781/head
Conduitry 5 years ago committed by GitHub
parent d91e9afab6
commit f68b3a3b8c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 52 additions and 32 deletions
Show Stats Download Patch File Download Diff File

1
CHANGELOG.md
Unescape View File

@ -18,6 +18,7 @@
* Throw exception immediately when calling `createEventDispatcher()` after component instantiation ([#3667](https://github.com/sveltejs/svelte/pull/3667))
* Fix globals shadowing contextual template scope ([#3674](https://github.com/sveltejs/svelte/issues/3674))
* Fix error resulting from trying to set a read-only property when spreading element attributes ([#3681](https://github.com/sveltejs/svelte/issues/3681))
* Fix handling of boolean attributes in presence of other spread attributes ([#3764](https://github.com/sveltejs/svelte/issues/3764))
## 3.12.1

2
src/compiler/compile/nodes/Attribute.ts
Unescape View File

@ -9,7 +9,7 @@ import TemplateScope from './shared/TemplateScope';
import { x } from 'code-red';
export default class Attribute extends Node {
type: 'Attribute';
type: 'Attribute' | 'Spread';
start: number;
end: number;
scope: TemplateScope;

11
src/compiler/compile/render_dom/wrappers/Element/Attribute.ts
Unescape View File

@ -44,9 +44,7 @@ export default class AttributeWrapper {
const element = this.parent;
const name = fix_attribute_casing(this.node.name);
let metadata = element.node.namespace ? null : attribute_lookup[name];
if (metadata && metadata.applies_to && !~metadata.applies_to.indexOf(element.node.name))
metadata = null;
const metadata = this.get_metadata();
const is_indirectly_bound_value =
name === 'value' &&
@ -193,6 +191,13 @@ export default class AttributeWrapper {
}
}
get_metadata() {
if (this.parent.node.namespace) return null;
const metadata = attribute_lookup[fix_attribute_casing(this.node.name)];
if (metadata && metadata.applies_to && !metadata.applies_to.includes(this.parent.node.name)) return null;
return metadata;
}
get_class_name_text() {
const scoped_css = this.node.chunks.some((chunk: Text) => chunk.synthetic);
const rendered = this.render_chunks();

20
src/compiler/compile/render_dom/wrappers/Element/index.ts
Unescape View File

@ -573,8 +573,7 @@ export default class ElementWrapper extends Wrapper {
}
});
// @ts-ignore todo:
if (this.node.attributes.find(attr => attr.type === 'Spread')) {
if (this.node.attributes.some(attr => attr.is_spread)) {
this.add_spread_attributes(block);
return;
}
@ -591,21 +590,24 @@ export default class ElementWrapper extends Wrapper {
const initial_props = [];
const updates = [];
this.node.attributes
.filter(attr => attr.type === 'Attribute' || attr.type === 'Spread')
this.attributes
.forEach(attr => {
const condition = attr.dependencies.size > 0
? changed(Array.from(attr.dependencies))
const condition = attr.node.dependencies.size > 0
? changed(Array.from(attr.node.dependencies))
: null;
if (attr.is_spread) {
const snippet = attr.expression.manipulate(block);
if (attr.node.is_spread) {
const snippet = attr.node.expression.manipulate(block);
initial_props.push(snippet);
updates.push(condition ? x`${condition} && ${snippet}` : snippet);
} else {
const snippet = x`{ ${attr.name}: ${attr.get_value(block)} }`;
const metadata = attr.get_metadata();
const snippet = x`{ ${
(metadata && metadata.property_name) ||
fix_attribute_casing(attr.node.name)
}: ${attr.node.get_value(block)} }`;
initial_props.push(snippet);
updates.push(condition ? x`${condition} && ${snippet}` : snippet);

32
src/compiler/compile/render_ssr/handlers/Element.ts
Unescape View File

@ -1,5 +1,4 @@
import { is_void } from '../../../utils/names';
import Attribute from '../../nodes/Attribute';
import Class from '../../nodes/Class';
import { get_attribute_value, get_class_attribute_value } from './shared/get_attribute_value';
import { get_slot_scope } from './shared/get_slot_scope';
@ -80,62 +79,61 @@ export default function(node: Element, renderer: Renderer, options: RenderOption
let add_class_attribute = class_expression ? true : false;
if (node.attributes.find(attr => attr.is_spread)) {
if (node.attributes.some(attr => attr.is_spread)) {
// TODO dry this out
const args = [];
node.attributes.forEach(attribute => {
if (attribute.is_spread) {
args.push(attribute.expression.node);
} else {
if (attribute.name === 'value' && node.name === 'textarea') {
const name = attribute.name.toLowerCase();
if (name === 'value' && node.name.toLowerCase() === 'textarea') {
node_contents = get_attribute_value(attribute);
} else if (attribute.is_true) {
args.push(x`{ ${attribute.name}: true }`);
} else if (
boolean_attributes.has(attribute.name) &&
boolean_attributes.has(name) &&
attribute.chunks.length === 1 &&
attribute.chunks[0].type !== 'Text'
) {
// a boolean attribute with one non-Text chunk
args.push(x`{ ${attribute.name}: ${(attribute.chunks[0] as Expression).node} }`);
} else if (attribute.name === 'class' && class_expression) {
args.push(x`{ ${attribute.name}: ${(attribute.chunks[0] as Expression).node} || null }`);
} else if (name === 'class' && class_expression) {
// Add class expression
args.push(x`{ ${attribute.name}: [${get_class_attribute_value(attribute)}, ${class_expression}].join(' ').trim() }`);
} else {
args.push(x`{ ${attribute.name}: ${attribute.name === 'class' ? get_class_attribute_value(attribute) : get_attribute_value(attribute)} }`);
args.push(x`{ ${attribute.name}: ${(name === 'class' ? get_class_attribute_value : get_attribute_value)(attribute)} }`);
}
}
});
renderer.add_expression(x`@spread([${args}])`);
} else {
node.attributes.forEach((attribute: Attribute) => {
if (attribute.type !== 'Attribute') return;
if (attribute.name === 'value' && node.name === 'textarea') {
node.attributes.forEach(attribute => {
const name = attribute.name.toLowerCase();
if (name === 'value' && node.name.toLowerCase() === 'textarea') {
node_contents = get_attribute_value(attribute);
} else if (attribute.is_true) {
renderer.add_string(` ${attribute.name}`);
} else if (
boolean_attributes.has(attribute.name) &&
boolean_attributes.has(name) &&
attribute.chunks.length === 1 &&
attribute.chunks[0].type !== 'Text'
) {
// a boolean attribute with one non-Text chunk
renderer.add_string(` `);
renderer.add_expression(x`${(attribute.chunks[0] as Expression).node} ? "${attribute.name}" : ""`);
} else if (attribute.name === 'class' && class_expression) {
} else if (name === 'class' && class_expression) {
add_class_attribute = false;
renderer.add_string(` class="`);
renderer.add_string(` ${attribute.name}="`);
renderer.add_expression(x`[${get_class_attribute_value(attribute)}, ${class_expression}].join(' ').trim()`);
renderer.add_string(`"`);
} else if (attribute.chunks.length === 1 && attribute.chunks[0].type !== 'Text') {
const { name } = attribute;
const snippet = (attribute.chunks[0] as Expression).node;
renderer.add_expression(x`@add_attribute("${name}", ${snippet}, ${boolean_attributes.has(name) ? 1 : 0})`);
renderer.add_expression(x`@add_attribute("${attribute.name}", ${snippet}, ${boolean_attributes.has(name) ? 1 : 0})`);
} else {
renderer.add_string(` ${attribute.name}="`);
renderer.add_expression(attribute.name === 'class' ? get_class_attribute_value(attribute) : get_attribute_value(attribute));
renderer.add_expression((name === 'class' ? get_class_attribute_value : get_attribute_value)(attribute));
renderer.add_string(`"`);
}
});

4
src/runtime/internal/dom.ts
Unescape View File

@ -93,7 +93,9 @@ export function set_attributes(node: Element & ElementCSSInlineStyle, attributes
// @ts-ignore
const descriptors = Object.getOwnPropertyDescriptors(node.__proto__);
for (const key in attributes) {
if (key === 'style') {
if (attributes[key] == null) {
node.removeAttribute(key);
} else if (key === 'style') {
node.style.cssText = attributes[key];
} else if (descriptors[key] && descriptors[key].set) {
node[key] = attributes[key];

2
src/runtime/internal/ssr.ts
Unescape View File

@ -13,7 +13,7 @@ export function spread(args) {
if (invalid_attribute_name_character.test(name)) return;
const value = attributes[name];
if (value === undefined) return;
if (value == null) return;
if (value === true) str += " " + name;
const escaped = String(value)

3
test/runtime/samples/attribute-boolean-case-insensitive/_config.js
Unescape View File

@ -0,0 +1,3 @@
export default {
html: `<input readonly>`
};

1
test/runtime/samples/attribute-boolean-case-insensitive/main.svelte
Unescape View File

@ -0,0 +1 @@
<input READONLY={true} REQUIRED={false}>

3
test/runtime/samples/attribute-boolean-with-spread/_config.js
Unescape View File

@ -0,0 +1,3 @@
export default {
html: `<input>`
};

1
test/runtime/samples/attribute-boolean-with-spread/main.svelte
Unescape View File

@ -0,0 +1 @@
<input {...{ foo: null }} readonly={false} required={false} disabled={null}>

3
test/runtime/samples/spread-element-removal/_config.js
Unescape View File

@ -0,0 +1,3 @@
export default {
html: `<input>`
};

1
test/runtime/samples/spread-element-removal/main.svelte
Unescape View File

@ -0,0 +1 @@
<input placeholder='foo' {...{ placeholder: null }}>
Write Preview
Loading…
Cancel
Save