From 32b55eaa936b81c7490fd801b4730d97d816d8df Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Wed, 17 Jul 2024 21:41:35 +0200 Subject: [PATCH] breaking: warn on quotes single-expression attributes in runes mode (#12479) * parse `foo={bar}` attribute as `value: { type: 'ExpressionTag', .. }` (i.e. don't wrap in an array) * warn on quoted single-expression-attributes * breaking: warn on quotes single-expression attributes in runes mode In a future version, that will mean things are getting stringified, which is a departure from how things work today, therefore a warning first. Related to #7925 * Update .changeset/plenty-items-build.md * Apply suggestions from code review * missed a spot --------- Co-authored-by: Rich Harris --- .changeset/plenty-items-build.md | 5 ++ .../messages/compile-warnings/template.md | 4 ++ packages/svelte/src/compiler/legacy.js | 28 +++++++++++ packages/svelte/src/compiler/migrate/index.js | 8 +-- .../compiler/phases/1-parse/read/options.js | 10 +++- .../compiler/phases/1-parse/state/element.js | 37 ++++++++------ .../phases/2-analyze/css/css-prune.js | 10 ++-- .../src/compiler/phases/2-analyze/index.js | 28 +++++++---- .../compiler/phases/2-analyze/validation.js | 34 ++++++++++--- .../3-transform/client/visitors/template.js | 44 ++++++++-------- .../3-transform/server/transform-server.js | 21 +++++--- packages/svelte/src/compiler/phases/nodes.js | 2 +- .../src/compiler/types/legacy-nodes.d.ts | 12 ++++- .../svelte/src/compiler/types/template.d.ts | 4 +- packages/svelte/src/compiler/utils/ast.js | 39 ++++++++++++--- packages/svelte/src/compiler/warnings.js | 9 ++++ .../parser-modern/samples/options/output.json | 50 +++++++++---------- .../samples/attribute-quoted/input.svelte | 21 ++++++++ .../samples/attribute-quoted/warnings.json | 50 +++++++++++++++++++ packages/svelte/types/index.d.ts | 16 ++++-- 20 files changed, 320 insertions(+), 112 deletions(-) create mode 100644 .changeset/plenty-items-build.md create mode 100644 packages/svelte/tests/validator/samples/attribute-quoted/input.svelte create mode 100644 packages/svelte/tests/validator/samples/attribute-quoted/warnings.json diff --git a/.changeset/plenty-items-build.md b/.changeset/plenty-items-build.md new file mode 100644 index 0000000000..93ec30324d --- /dev/null +++ b/.changeset/plenty-items-build.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +breaking: warn on quoted single-expression attributes in runes mode diff --git a/packages/svelte/messages/compile-warnings/template.md b/packages/svelte/messages/compile-warnings/template.md index 9cf98b8c88..3460e1098e 100644 --- a/packages/svelte/messages/compile-warnings/template.md +++ b/packages/svelte/messages/compile-warnings/template.md @@ -14,6 +14,10 @@ > '%wrong%' is not a valid HTML attribute. Did you mean '%right%'? +## attribute_quoted + +> Quoted attributes on components and custom elements will be stringified in a future version of Svelte. If this isn't what you want, remove the quotes + ## bind_invalid_each_rest > The rest operator (...) will create a new object and binding '%name%' with the original object will not work diff --git a/packages/svelte/src/compiler/legacy.js b/packages/svelte/src/compiler/legacy.js index 1525522d98..9d4050eb62 100644 --- a/packages/svelte/src/compiler/legacy.js +++ b/packages/svelte/src/compiler/legacy.js @@ -411,6 +411,34 @@ export function convert(source, ast) { ) }; }, + Attribute(node, { visit, next, path }) { + if (node.value !== true && !Array.isArray(node.value)) { + path.push(node); + const value = /** @type {Legacy.LegacyAttribute['value']} */ ([visit(node.value)]); + path.pop(); + + return { + ...node, + value + }; + } else { + return next(); + } + }, + StyleDirective(node, { visit, next, path }) { + if (node.value !== true && !Array.isArray(node.value)) { + path.push(node); + const value = /** @type {Legacy.LegacyStyleDirective['value']} */ ([visit(node.value)]); + path.pop(); + + return { + ...node, + value + }; + } else { + return next(); + } + }, SpreadAttribute(node) { return { ...node, type: 'Spread' }; }, diff --git a/packages/svelte/src/compiler/migrate/index.js b/packages/svelte/src/compiler/migrate/index.js index dedc6e6d79..cc5344e5c2 100644 --- a/packages/svelte/src/compiler/migrate/index.js +++ b/packages/svelte/src/compiler/migrate/index.js @@ -532,11 +532,13 @@ const template = { if (attr.name === 'name') { slot_name = /** @type {any} */ (attr.value)[0].data; } else { + const attr_value = + attr.value === true || Array.isArray(attr.value) ? attr.value : [attr.value]; const value = - attr.value !== true + attr_value !== true ? state.str.original.substring( - attr.value[0].start, - attr.value[attr.value.length - 1].end + attr_value[0].start, + attr_value[attr_value.length - 1].end ) : 'true'; slot_props += value === attr.name ? `${value}, ` : `${attr.name}: ${value}, `; diff --git a/packages/svelte/src/compiler/phases/1-parse/read/options.js b/packages/svelte/src/compiler/phases/1-parse/read/options.js index d081828de6..27527dd162 100644 --- a/packages/svelte/src/compiler/phases/1-parse/read/options.js +++ b/packages/svelte/src/compiler/phases/1-parse/read/options.js @@ -41,8 +41,9 @@ export default function read_options(node) { case 'customElement': { /** @type {SvelteOptions['customElement']} */ const ce = { tag: '' }; + const { value: v } = attribute; + const value = v === true || Array.isArray(v) ? v : [v]; - const { value } = attribute; if (value === true) { e.svelte_options_invalid_customelement(attribute); } else if (value[0].type === 'Text') { @@ -199,7 +200,11 @@ export default function read_options(node) { */ function get_static_value(attribute) { const { value } = attribute; - const chunk = value[0]; + + if (value === true) return true; + + const chunk = Array.isArray(value) ? value[0] : value; + if (!chunk) return true; if (value.length > 1) { return null; @@ -208,6 +213,7 @@ function get_static_value(attribute) { if (chunk.expression.type !== 'Literal') { return null; } + return chunk.expression.value; } diff --git a/packages/svelte/src/compiler/phases/1-parse/state/element.js b/packages/svelte/src/compiler/phases/1-parse/state/element.js index dcb7b23723..161b5fbc61 100644 --- a/packages/svelte/src/compiler/phases/1-parse/state/element.js +++ b/packages/svelte/src/compiler/phases/1-parse/state/element.js @@ -9,6 +9,7 @@ import * as e from '../../../errors.js'; import * as w from '../../../warnings.js'; import { create_fragment } from '../utils/create.js'; import { create_attribute } from '../../nodes.js'; +import { get_attribute_expression, is_expression_attribute } from '../../../utils/ast.js'; // eslint-disable-next-line no-useless-escape const valid_tag_name = /^\!?[a-zA-Z]{1,}:?[a-zA-Z0-9\-]*/; @@ -241,15 +242,11 @@ export default function element(parser) { } const definition = /** @type {Compiler.Attribute} */ (element.attributes.splice(index, 1)[0]); - if ( - definition.value === true || - definition.value.length !== 1 || - definition.value[0].type === 'Text' - ) { + if (!is_expression_attribute(definition)) { e.svelte_component_invalid_this(definition.start); } - element.expression = definition.value[0].expression; + element.expression = get_attribute_expression(definition); } if (element.type === 'SvelteElement') { @@ -267,15 +264,16 @@ export default function element(parser) { e.svelte_element_missing_this(definition); } - const chunk = definition.value[0]; - - if (definition.value.length !== 1 || chunk.type !== 'ExpressionTag') { + if (!is_expression_attribute(definition)) { w.svelte_element_invalid_this(definition); // note that this is wrong, in the case of e.g. `this="h{n}"` — it will result in ``. // it would be much better to just error here, but we are preserving the existing buggy // Svelte 4 behaviour out of an overabundance of caution regarding breaking changes. // TODO in 6.0, error + const chunk = /** @type {Array} */ ( + definition.value + )[0]; element.tag = chunk.type === 'Text' ? { @@ -287,7 +285,7 @@ export default function element(parser) { } : chunk.expression; } else { - element.tag = chunk.expression; + element.tag = get_attribute_expression(definition); } } @@ -543,7 +541,7 @@ function read_attribute(parser) { } }; - return create_attribute(name, start, parser.index, [expression]); + return create_attribute(name, start, parser.index, expression); } } @@ -557,7 +555,7 @@ function read_attribute(parser) { const colon_index = name.indexOf(':'); const type = colon_index !== -1 && get_directive_type(name.slice(0, colon_index)); - /** @type {true | Array} */ + /** @type {true | Compiler.ExpressionTag | Array} */ let value = true; if (parser.eat('=')) { parser.allow_whitespace(); @@ -589,7 +587,9 @@ function read_attribute(parser) { }; } - const first_value = value === true ? undefined : value[0]; + const first_value = value === true ? undefined : Array.isArray(value) ? value[0] : value; + + /** @type {import('estree').Expression | null} */ let expression = null; if (first_value) { @@ -598,6 +598,8 @@ function read_attribute(parser) { if (attribute_contains_text) { e.directive_invalid_value(/** @type {number} */ (first_value.start)); } else { + // TODO throw a parser error in a future version here if this `[ExpressionTag]` instead of `ExpressionTag`, + // which means stringified value, which isn't allowed for some directives? expression = first_value.expression; } } @@ -662,6 +664,7 @@ function get_directive_type(name) { /** * @param {Parser} parser + * @return {Compiler.ExpressionTag | Array} */ function read_attribute_value(parser) { const quote_mark = parser.eat("'") ? "'" : parser.eat('"') ? '"' : null; @@ -678,6 +681,7 @@ function read_attribute_value(parser) { ]; } + /** @type {Array} */ let value; try { value = read_sequence( @@ -708,7 +712,12 @@ function read_attribute_value(parser) { } if (quote_mark) parser.index += 1; - return value; + + if (quote_mark || value.length > 1 || value[0].type === 'Text') { + return value; + } else { + return value[0]; + } } /** diff --git a/packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js b/packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js index 893f543a5f..f238e3e075 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js +++ b/packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js @@ -3,6 +3,7 @@ import { walk } from 'zimmerframe'; import { get_possible_values } from './utils.js'; import { regex_ends_with_whitespace, regex_starts_with_whitespace } from '../../patterns.js'; +import { get_attribute_chunks, is_text_attribute } from '../../../utils/ast.js'; /** * @typedef {{ @@ -444,14 +445,11 @@ function attribute_matches(node, name, expected_value, operator, case_insensitiv if (attribute.value === true) return operator === null; if (expected_value === null) return true; - const chunks = attribute.value; - if (chunks.length === 1) { - const value = chunks[0]; - if (value.type === 'Text') { - return test_attribute(operator, expected_value, case_insensitive, value.data); - } + if (is_text_attribute(attribute)) { + return test_attribute(operator, expected_value, case_insensitive, attribute.value[0].data); } + const chunks = get_attribute_chunks(attribute.value); const possible_values = new Set(); /** @type {string[]} */ diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index 4fefd327c6..a064a457d0 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -9,7 +9,9 @@ import { is_event_attribute, is_text_attribute, object, - unwrap_optional + unwrap_optional, + get_attribute_expression, + get_attribute_chunks } from '../../utils/ast.js'; import * as b from '../../utils/builders.js'; import { MathMLElements, ReservedKeywords, Runes, SVGElements } from '../constants.js'; @@ -597,19 +599,24 @@ export function analyze_component(root, source, options) { } if (class_attribute && class_attribute.value !== true) { - const chunks = class_attribute.value; - - if (chunks.length === 1 && chunks[0].type === 'Text') { - chunks[0].data += ` ${analysis.css.hash}`; + if (is_text_attribute(class_attribute)) { + class_attribute.value[0].data += ` ${analysis.css.hash}`; } else { - chunks.push({ + /** @type {import('#compiler').Text} */ + const css_text = { type: 'Text', data: ` ${analysis.css.hash}`, raw: ` ${analysis.css.hash}`, start: -1, end: -1, parent: null - }); + }; + + if (Array.isArray(class_attribute.value)) { + class_attribute.value.push(css_text); + } else { + class_attribute.value = [class_attribute.value, css_text]; + } } } else { element.attributes.push( @@ -1171,7 +1178,7 @@ const common_visitors = { context.next(); - node.metadata.dynamic = node.value.some((chunk) => { + node.metadata.dynamic = get_attribute_chunks(node.value).some((chunk) => { if (chunk.type !== 'ExpressionTag') { return false; } @@ -1192,8 +1199,7 @@ const common_visitors = { context.state.analysis.uses_event_attributes = true; } - const expression = node.value[0].expression; - + const expression = get_attribute_expression(node); const delegated_event = get_delegated_event(node.name.slice(2), expression, context); if (delegated_event !== null) { @@ -1228,7 +1234,7 @@ const common_visitors = { } } else { context.next(); - node.metadata.dynamic = node.value.some( + node.metadata.dynamic = get_attribute_chunks(node.value).some( (node) => node.type === 'ExpressionTag' && node.metadata.dynamic ); } diff --git a/packages/svelte/src/compiler/phases/2-analyze/validation.js b/packages/svelte/src/compiler/phases/2-analyze/validation.js index a5b22c666f..29758b6b2f 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/validation.js +++ b/packages/svelte/src/compiler/phases/2-analyze/validation.js @@ -7,6 +7,7 @@ import { import * as e from '../../errors.js'; import { extract_identifiers, + get_attribute_expression, get_parent, is_expression_attribute, is_text_attribute, @@ -33,9 +34,26 @@ import { Scope, get_rune } from '../scope.js'; import { merge } from '../visitors.js'; import { a11y_validators } from './a11y.js'; -/** @param {import('#compiler').Attribute} attribute */ -function validate_attribute(attribute) { - if (attribute.value === true || attribute.value.length === 1) return; +/** + * @param {import('#compiler').Attribute} attribute + * @param {import('#compiler').ElementLike} parent + */ +function validate_attribute(attribute, parent) { + if ( + Array.isArray(attribute.value) && + attribute.value.length === 1 && + attribute.value[0].type === 'ExpressionTag' && + (parent.type === 'Component' || + parent.type === 'SvelteComponent' || + parent.type === 'SvelteSelf' || + (parent.type === 'RegularElement' && is_custom_element_node(parent))) + ) { + w.attribute_quoted(attribute); + } + + if (attribute.value === true || !Array.isArray(attribute.value) || attribute.value.length === 1) { + return; + } const is_quoted = attribute.value.at(-1)?.end !== attribute.end; @@ -69,10 +87,10 @@ function validate_component(node, context) { if (attribute.type === 'Attribute') { if (context.state.analysis.runes) { - validate_attribute(attribute); + validate_attribute(attribute, node); if (is_expression_attribute(attribute)) { - const expression = attribute.value[0].expression; + const expression = get_attribute_expression(attribute); if (expression.type === 'SequenceExpression') { let i = /** @type {number} */ (expression.start); while (--i > 0) { @@ -122,10 +140,10 @@ function validate_element(node, context) { const is_expression = is_expression_attribute(attribute); if (context.state.analysis.runes) { - validate_attribute(attribute); + validate_attribute(attribute, node); if (is_expression) { - const expression = attribute.value[0].expression; + const expression = get_attribute_expression(attribute); if (expression.type === 'SequenceExpression') { let i = /** @type {number} */ (expression.start); while (--i > 0) { @@ -146,7 +164,7 @@ function validate_element(node, context) { e.attribute_invalid_event_handler(attribute); } - const value = attribute.value[0].expression; + const value = get_attribute_expression(attribute); if ( value.type === 'Identifier' && value.name === attribute.name && diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js index 573efe211a..f5bd0af8da 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js @@ -2,6 +2,8 @@ import { extract_identifiers, extract_paths, + get_attribute_chunks, + get_attribute_expression, is_event_attribute, is_text_attribute, object, @@ -96,11 +98,9 @@ function serialize_style_directives(style_directives, element_id, context, is_at ) ); - const contains_call_expression = - Array.isArray(directive.value) && - directive.value.some( - (v) => v.type === 'ExpressionTag' && v.metadata.contains_call_expression - ); + const contains_call_expression = get_attribute_chunks(directive.value).some( + (v) => v.type === 'ExpressionTag' && v.metadata.contains_call_expression + ); if (!is_attributes_reactive && contains_call_expression) { state.init.push(serialize_update(update)); @@ -286,8 +286,8 @@ function serialize_element_spread_attributes( if ( is_event_attribute(attribute) && - (attribute.value[0].expression.type === 'ArrowFunctionExpression' || - attribute.value[0].expression.type === 'FunctionExpression') + (get_attribute_expression(attribute).type === 'ArrowFunctionExpression' || + get_attribute_expression(attribute).type === 'FunctionExpression') ) { // Give the event handler a stable ID so it isn't removed and readded on every update const id = context.state.scope.generate('event_handler'); @@ -385,8 +385,8 @@ function serialize_dynamic_element_attributes(attributes, context, element_id) { if ( is_event_attribute(attribute) && - (attribute.value[0].expression.type === 'ArrowFunctionExpression' || - attribute.value[0].expression.type === 'FunctionExpression') + (get_attribute_expression(attribute).type === 'ArrowFunctionExpression' || + get_attribute_expression(attribute).type === 'FunctionExpression') ) { // Give the event handler a stable ID so it isn't removed and readded on every update const id = context.state.scope.generate('event_handler'); @@ -757,15 +757,13 @@ function serialize_inline_component(node, component_name, context, anchor = cont // When we have a non-simple computation, anything other than an Identifier or Member expression, // then there's a good chance it needs to be memoized to avoid over-firing when read within the // child component. - const should_wrap_in_derived = - Array.isArray(attribute.value) && - attribute.value.some((n) => { - return ( - n.type === 'ExpressionTag' && - n.expression.type !== 'Identifier' && - n.expression.type !== 'MemberExpression' - ); - }); + const should_wrap_in_derived = get_attribute_chunks(attribute.value).some((n) => { + return ( + n.type === 'ExpressionTag' && + n.expression.type !== 'Identifier' && + n.expression.type !== 'MemberExpression' + ); + }); if (should_wrap_in_derived) { const id = b.id(context.state.scope.generate(attribute.name)); @@ -1291,7 +1289,7 @@ function serialize_event(node, context) { } /** - * @param {import('#compiler').Attribute & { value: [import('#compiler').ExpressionTag] }} node + * @param {import('#compiler').Attribute & { value: import('#compiler').ExpressionTag | [import('#compiler').ExpressionTag] }} node * @param {import('../types').ComponentContext} context */ function serialize_event_attribute(node, context) { @@ -1307,7 +1305,7 @@ function serialize_event_attribute(node, context) { serialize_event( { name: event_name, - expression: node.value[0].expression, + expression: get_attribute_expression(node), modifiers, delegated: node.metadata.delegated }, @@ -1468,7 +1466,7 @@ function get_node_id(expression, state, name) { } /** - * @param {true | Array} value + * @param {import('#compiler').Attribute['value']} value * @param {import('../types').ComponentContext} context * @returns {[contains_call_expression: boolean, Expression]} */ @@ -1477,8 +1475,8 @@ function serialize_attribute_value(value, context) { return [false, b.literal(true)]; } - if (value.length === 1) { - const chunk = value[0]; + if (!Array.isArray(value) || value.length === 1) { + const chunk = Array.isArray(value) ? value[0] : value; if (chunk.type === 'Text') { return [false, b.literal(chunk.data)]; diff --git a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js index 3038316ede..04115a850e 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js @@ -3,6 +3,7 @@ import { set_scope, get_rune } from '../../scope.js'; import { extract_identifiers, extract_paths, + get_attribute_chunks, is_event_attribute, is_expression_async, is_text_attribute, @@ -678,7 +679,7 @@ const javascript_visitors_runes = { /** * - * @param {true | Array} value + * @param {import('#compiler').Attribute['value']} value * @param {import('./types').ComponentContext} context * @param {boolean} trim_whitespace * @param {boolean} is_component @@ -689,8 +690,8 @@ function serialize_attribute_value(value, context, trim_whitespace = false, is_c return b.true; } - if (value.length === 1) { - const chunk = value[0]; + if (!Array.isArray(value) || value.length === 1) { + const chunk = Array.isArray(value) ? value[0] : value; if (chunk.type === 'Text') { const data = trim_whitespace @@ -1667,6 +1668,7 @@ function serialize_element_attributes(node, context) { if (node.name === 'textarea') { if ( attribute.value !== true && + Array.isArray(attribute.value) && attribute.value[0].type === 'Text' && regex_starts_with_newline.test(attribute.value[0].data) ) { @@ -1891,15 +1893,19 @@ function serialize_class_directives(class_directives, class_attribute) { const expressions = class_directives.map((directive) => b.conditional(directive.expression, b.literal(directive.name), b.literal('')) ); + if (class_attribute === null) { class_attribute = create_attribute('class', -1, -1, []); } - const last = /** @type {any[]} */ (class_attribute.value).at(-1); + + const chunks = get_attribute_chunks(class_attribute.value); + const last = chunks.at(-1); + if (last?.type === 'Text') { last.data += ' '; last.raw += ' '; } else if (last) { - /** @type {import('#compiler').Text[]} */ (class_attribute.value).push({ + chunks.push({ type: 'Text', start: -1, end: -1, @@ -1908,7 +1914,8 @@ function serialize_class_directives(class_directives, class_attribute) { raw: ' ' }); } - /** @type {import('#compiler').ExpressionTag[]} */ (class_attribute.value).push({ + + chunks.push({ type: 'ExpressionTag', start: -1, end: -1, @@ -1922,6 +1929,8 @@ function serialize_class_directives(class_directives, class_attribute) { ), metadata: { contains_call_expression: false, dynamic: false } }); + + class_attribute.value = chunks; return class_attribute; } diff --git a/packages/svelte/src/compiler/phases/nodes.js b/packages/svelte/src/compiler/phases/nodes.js index e8843bc423..bb69b9749a 100644 --- a/packages/svelte/src/compiler/phases/nodes.js +++ b/packages/svelte/src/compiler/phases/nodes.js @@ -33,7 +33,7 @@ export function is_custom_element_node(node) { * @param {string} name * @param {number} start * @param {number} end - * @param {true | Array} value + * @param {Compiler.Attribute['value']} value * @returns {Compiler.Attribute} */ export function create_attribute(name, start, end, value) { diff --git a/packages/svelte/src/compiler/types/legacy-nodes.d.ts b/packages/svelte/src/compiler/types/legacy-nodes.d.ts index 0ba61c053c..c19f2b1c96 100644 --- a/packages/svelte/src/compiler/types/legacy-nodes.d.ts +++ b/packages/svelte/src/compiler/types/legacy-nodes.d.ts @@ -1,4 +1,4 @@ -import type { StyleDirective as LegacyStyleDirective, Text, Css } from '#compiler'; +import type { Text, Css, ExpressionTag } from '#compiler'; import type { ArrayExpression, AssignmentExpression, @@ -194,6 +194,16 @@ export interface LegacyTransition extends BaseNode { outro: boolean; } +/** A `style:` directive */ +export interface LegacyStyleDirective extends BaseNode { + type: 'StyleDirective'; + /** The 'x' in `style:x` */ + name: string; + /** The 'y' in `style:x={y}` */ + value: true | Array; + modifiers: Array<'important'>; +} + export interface LegacyWindow extends BaseElement { type: 'Window'; } diff --git a/packages/svelte/src/compiler/types/template.d.ts b/packages/svelte/src/compiler/types/template.d.ts index c5ec44e1df..8812f2fd9c 100644 --- a/packages/svelte/src/compiler/types/template.d.ts +++ b/packages/svelte/src/compiler/types/template.d.ts @@ -226,7 +226,7 @@ export interface StyleDirective extends BaseNode { /** The 'x' in `style:x` */ name: string; /** The 'y' in `style:x={y}` */ - value: true | Array; + value: true | ExpressionTag | Array; modifiers: Array<'important'>; metadata: { dynamic: boolean; @@ -447,7 +447,7 @@ export type Block = EachBlock | IfBlock | AwaitBlock | KeyBlock | SnippetBlock; export interface Attribute extends BaseNode { type: 'Attribute'; name: string; - value: true | Array; + value: true | ExpressionTag | Array; metadata: { dynamic: boolean; /** May be set if this is an event attribute */ diff --git a/packages/svelte/src/compiler/utils/ast.js b/packages/svelte/src/compiler/utils/ast.js index f122ed5021..7d786cb5fe 100644 --- a/packages/svelte/src/compiler/utils/ast.js +++ b/packages/svelte/src/compiler/utils/ast.js @@ -27,27 +27,54 @@ export function object(expression) { */ export function is_text_attribute(attribute) { return ( - attribute.value !== true && attribute.value.length === 1 && attribute.value[0].type === 'Text' + Array.isArray(attribute.value) && + attribute.value.length === 1 && + attribute.value[0].type === 'Text' ); } /** * Returns true if the attribute contains a single expression node. + * In Svelte 5, this also includes a single expression node wrapped in an array. + * TODO change that in a future version * @param {Attribute} attribute - * @returns {attribute is Attribute & { value: [ExpressionTag] }} + * @returns {attribute is Attribute & { value: [ExpressionTag] | ExpressionTag }} */ export function is_expression_attribute(attribute) { return ( - attribute.value !== true && - attribute.value.length === 1 && - attribute.value[0].type === 'ExpressionTag' + (attribute.value !== true && !Array.isArray(attribute.value)) || + (Array.isArray(attribute.value) && + attribute.value.length === 1 && + attribute.value[0].type === 'ExpressionTag') ); } +/** + * Returns the single attribute expression node. + * In Svelte 5, this also includes a single expression node wrapped in an array. + * TODO change that in a future version + * @param { Attribute & { value: [ExpressionTag] | ExpressionTag }} attribute + * @returns {ESTree.Expression} + */ +export function get_attribute_expression(attribute) { + return Array.isArray(attribute.value) + ? /** @type {ExpressionTag} */ (attribute.value[0]).expression + : attribute.value.expression; +} + +/** + * Returns the expression chunks of an attribute value + * @param {Attribute['value']} value + * @returns {Array} + */ +export function get_attribute_chunks(value) { + return Array.isArray(value) ? value : typeof value === 'boolean' ? [] : [value]; +} + /** * Returns true if the attribute starts with `on` and contains a single expression node. * @param {Attribute} attribute - * @returns {attribute is Attribute & { value: [ExpressionTag] }} + * @returns {attribute is Attribute & { value: [ExpressionTag] | ExpressionTag }} */ export function is_event_attribute(attribute) { return is_expression_attribute(attribute) && attribute.name.startsWith('on'); diff --git a/packages/svelte/src/compiler/warnings.js b/packages/svelte/src/compiler/warnings.js index 7396bf40fb..d9c197cf75 100644 --- a/packages/svelte/src/compiler/warnings.js +++ b/packages/svelte/src/compiler/warnings.js @@ -108,6 +108,7 @@ export const codes = [ "attribute_global_event_reference", "attribute_illegal_colon", "attribute_invalid_property_name", + "attribute_quoted", "bind_invalid_each_rest", "block_empty", "component_name_lowercase", @@ -686,6 +687,14 @@ export function attribute_invalid_property_name(node, wrong, right) { w(node, "attribute_invalid_property_name", `'${wrong}' is not a valid HTML attribute. Did you mean '${right}'?`); } +/** + * Quoted attributes on components and custom elements will be stringified in a future version of Svelte. If this isn't what you want, remove the quotes + * @param {null | NodeLike} node + */ +export function attribute_quoted(node) { + w(node, "attribute_quoted", "Quoted attributes on components and custom elements will be stringified in a future version of Svelte. If this isn't what you want, remove the quotes"); +} + /** * The rest operator (...) will create a new object and binding '%name%' with the original object will not work * @param {null | NodeLike} node diff --git a/packages/svelte/tests/parser-modern/samples/options/output.json b/packages/svelte/tests/parser-modern/samples/options/output.json index 398c178cb4..708bf0cd1a 100644 --- a/packages/svelte/tests/parser-modern/samples/options/output.json +++ b/packages/svelte/tests/parser-modern/samples/options/output.json @@ -54,35 +54,33 @@ "start": 50, "end": 62, "name": "runes", - "value": [ - { - "type": "ExpressionTag", - "start": 56, - "end": 62, - "expression": { - "type": "Literal", - "start": 57, - "end": 61, - "loc": { - "start": { - "line": 1, - "column": 57 - }, - "end": { - "line": 1, - "column": 61 - } + "value": { + "type": "ExpressionTag", + "start": 56, + "end": 62, + "expression": { + "type": "Literal", + "start": 57, + "end": 61, + "loc": { + "start": { + "line": 1, + "column": 57 }, - "value": true, - "raw": "true" + "end": { + "line": 1, + "column": 61 + } }, - "parent": null, - "metadata": { - "contains_call_expression": false, - "dynamic": false - } + "value": true, + "raw": "true" + }, + "parent": null, + "metadata": { + "contains_call_expression": false, + "dynamic": false } - ], + }, "parent": null, "metadata": { "dynamic": false, diff --git a/packages/svelte/tests/validator/samples/attribute-quoted/input.svelte b/packages/svelte/tests/validator/samples/attribute-quoted/input.svelte new file mode 100644 index 0000000000..9252d9b19a --- /dev/null +++ b/packages/svelte/tests/validator/samples/attribute-quoted/input.svelte @@ -0,0 +1,21 @@ + + + + + +

+ + + + + + + + + + +{#if foo} + +{/if} + + diff --git a/packages/svelte/tests/validator/samples/attribute-quoted/warnings.json b/packages/svelte/tests/validator/samples/attribute-quoted/warnings.json new file mode 100644 index 0000000000..3411a14cbe --- /dev/null +++ b/packages/svelte/tests/validator/samples/attribute-quoted/warnings.json @@ -0,0 +1,50 @@ +[ + { + "code": "attribute_quoted", + "message": "Quoted attributes on components and custom elements will be stringified in a future version of Svelte. If this isn't what you want, remove the quotes", + "start": { + "column": 11, + "line": 13 + }, + "end": { + "column": 24, + "line": 13 + } + }, + { + "code": "attribute_quoted", + "message": "Quoted attributes on components and custom elements will be stringified in a future version of Svelte. If this isn't what you want, remove the quotes", + "start": { + "column": 29, + "line": 15 + }, + "end": { + "column": 42, + "line": 15 + } + }, + { + "code": "attribute_quoted", + "message": "Quoted attributes on components and custom elements will be stringified in a future version of Svelte. If this isn't what you want, remove the quotes", + "start": { + "column": 14, + "line": 18 + }, + "end": { + "column": 27, + "line": 18 + } + }, + { + "code": "attribute_quoted", + "message": "Quoted attributes on components and custom elements will be stringified in a future version of Svelte. If this isn't what you want, remove the quotes", + "start": { + "column": 16, + "line": 21 + }, + "end": { + "column": 29, + "line": 21 + } + } +] diff --git a/packages/svelte/types/index.d.ts b/packages/svelte/types/index.d.ts index b0c8c026d3..86f33588a6 100644 --- a/packages/svelte/types/index.d.ts +++ b/packages/svelte/types/index.d.ts @@ -1153,6 +1153,16 @@ declare module 'svelte/compiler' { outro: boolean; } + /** A `style:` directive */ + interface LegacyStyleDirective extends BaseNode_1 { + type: 'StyleDirective'; + /** The 'x' in `style:x` */ + name: string; + /** The 'y' in `style:x={y}` */ + value: true | Array; + modifiers: Array<'important'>; + } + interface LegacyWindow extends BaseElement_1 { type: 'Window'; } @@ -1171,7 +1181,7 @@ declare module 'svelte/compiler' { | LegacyClass | LegacyLet | LegacyEventHandler - | StyleDirective + | LegacyStyleDirective | LegacyTransition | LegacyAction; @@ -1634,7 +1644,7 @@ declare module 'svelte/compiler' { /** The 'x' in `style:x` */ name: string; /** The 'y' in `style:x={y}` */ - value: true | Array; + value: true | ExpressionTag | Array; modifiers: Array<'important'>; metadata: { dynamic: boolean; @@ -1855,7 +1865,7 @@ declare module 'svelte/compiler' { interface Attribute extends BaseNode { type: 'Attribute'; name: string; - value: true | Array; + value: true | ExpressionTag | Array; metadata: { dynamic: boolean; /** May be set if this is an event attribute */