Skip to content

Commit 02ea592

Browse files
jackgdlljackgdll
committed
better error reporting
1 parent a9ffe15 commit 02ea592

File tree

15 files changed

+152
-60
lines changed

15 files changed

+152
-60
lines changed

‎packages/svelte/src/compiler/phases/1-parse/state/element.js‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/** @import { Expression } from 'estree' */
1+
/** @import { Expression, SpreadElement } from 'estree' */
22
/** @import { AST } from '#compiler' */
33
/** @import { Parser } from '../index.js' */
44
import { is_void } from '../../../../utils.js';
@@ -643,7 +643,7 @@ function read_attribute(parser) {
643643

644644
const first_value = value === true ? undefined : Array.isArray(value) ? value[0] : value;
645645

646-
/** @type {Expression | null} */
646+
/** @type {Expression | SpreadElement | null} */
647647
let expression = null;
648648

649649
if (first_value) {
@@ -655,10 +655,8 @@ function read_attribute(parser) {
655655
// TODO throw a parser error in a future version here if this `[ExpressionTag]` instead of `ExpressionTag`,
656656
// which means stringified value, which isn't allowed for some directives?
657657
expression = first_value.expression;
658-
659-
// Handle spread syntax in bind directives
658+
660659
if (type === 'BindDirective' && first_value.metadata.expression.has_spread) {
661-
// Create a SpreadElement to represent ...array syntax
662660
expression = {
663661
type: 'SpreadElement',
664662
start: first_value.start,

‎packages/svelte/src/compiler/phases/2-analyze/visitors/BindDirective.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,8 +167,8 @@ export function BindDirective(node, context) {
167167
// Validate that the spread is applied to a valid expression that returns an array
168168
const argument = node.expression.argument;
169169
if (
170-
argument.type !== 'Identifier' &&
171-
argument.type !== 'MemberExpression' &&
170+
argument.type !== 'Identifier' &&
171+
argument.type !== 'MemberExpression' &&
172172
argument.type !== 'CallExpression'
173173
) {
174174
e.bind_invalid_expression(node);

‎packages/svelte/src/compiler/phases/3-transform/client/visitors/BindDirective.js‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import * as b from '#compiler/builders';
77
import { binding_properties } from '../../../bindings.js';
88
import { build_attribute_value } from './shared/element.js';
99
import { build_bind_this, validate_binding } from './shared/utils.js';
10-
import { handle_spread_binding } from '../../shared/spread_bindings.js';
10+
import { init_spread_bindings } from '../../shared/spread_bindings.js';
1111

1212
/**
1313
* @param {AST.BindDirective} node
@@ -18,11 +18,7 @@ export function BindDirective(node, context) {
1818

1919
// Handle SpreadElement by creating a variable declaration before visiting
2020
if (node.expression.type === 'SpreadElement') {
21-
const { get: getter, set: setter } = handle_spread_binding(
22-
node.expression,
23-
context.state,
24-
context.visit
25-
);
21+
const { get: getter, set: setter } = init_spread_bindings(node.expression, context);
2622
get = getter;
2723
set = setter;
2824
} else {

‎packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { regex_is_valid_identifier } from '../../../../patterns.js';
99
import is_reference from 'is-reference';
1010
import { dev, is_ignored, locator, component_name } from '../../../../../state.js';
1111
import { build_getter } from '../../utils.js';
12-
import { handle_spread_binding } from '../../../shared/spread_bindings.js';
12+
import { init_spread_bindings } from '../../../shared/spread_bindings.js';
1313

1414
/**
1515
* A utility for extracting complex expressions (such as call expressions)
@@ -209,9 +209,10 @@ export function parse_directive_name(name) {
209209
* @param {Expression} value
210210
* @param {import('zimmerframe').Context<AST.SvelteNode, ComponentClientTransformState>} context
211211
*/
212-
export function build_bind_this(expression, value, { state, visit }) {
212+
export function build_bind_this(expression, value, context) {
213+
const { state, visit } = context;
213214
if (expression.type === 'SpreadElement') {
214-
const { get, set } = handle_spread_binding(expression, state, visit);
215+
const { get, set } = init_spread_bindings(expression, context);
215216
return b.call('$.bind_this', value, set, get);
216217
}
217218

‎packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import {
2121
is_load_error_element
2222
} from '../../../../../../utils.js';
2323
import { escape_html } from '../../../../../../escaping.js';
24-
import { handle_spread_binding } from '../../../shared/spread_bindings.js';
24+
import { init_spread_bindings } from '../../../shared/spread_bindings.js';
2525

2626
const WHITESPACE_INSENSITIVE_ATTRIBUTES = ['class', 'style'];
2727

@@ -121,7 +121,7 @@ export function build_element_attributes(node, context) {
121121

122122
// Handle SpreadElement for bind directives
123123
if (attribute.expression.type === 'SpreadElement') {
124-
const { get } = handle_spread_binding(attribute.expression, context.state, context.visit);
124+
const { get } = init_spread_bindings(attribute.expression, context);
125125
expression = b.call(get);
126126
} else if (expression.type === 'SequenceExpression') {
127127
expression = b.call(expression.expressions[0]);

‎packages/svelte/src/compiler/phases/3-transform/shared/spread_bindings.js‎

Lines changed: 19 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,49 +1,32 @@
1-
/** @import { CallExpression, Expression, SpreadElement, Super } from 'estree' */
1+
/** @import { Expression, SpreadElement } from 'estree' */
2+
/** @import { Context } from 'zimmerframe' */
23
/** @import { ComponentClientTransformState } from '../client/types.js' */
34
/** @import { ComponentServerTransformState } from '../server/types.js' */
5+
/** @import { AST } from '#compiler' */
46
import * as b from '#compiler/builders';
7+
import { dev, source } from '../../../state.js';
58

69
/**
7-
* Handles SpreadElement by creating a variable declaration and returning getter/setter expressions
10+
* Initializes spread bindings for a SpreadElement in a bind directive.
811
* @param {SpreadElement} spread_expression
9-
* @param {ComponentClientTransformState | ComponentServerTransformState} state
10-
* @param {function} visit
11-
* @returns {{get: Expression, set: Expression}}
12+
* @param {Context<AST.SvelteNode, ComponentClientTransformState> | Context<AST.SvelteNode, ComponentServerTransformState>} context
13+
* @returns {{ get: Expression, set: Expression }}
1214
*/
13-
export function handle_spread_binding(spread_expression, state, visit) {
14-
// Generate a unique variable name for this spread binding
15-
const id = b.id(state.scope.generate('$$bindings'));
16-
15+
export function init_spread_bindings(spread_expression, { state, visit }) {
1716
const visited_expression = /** @type {Expression} */ (visit(spread_expression.argument));
18-
state.init.push(b.const(id, visited_expression));
19-
20-
const noop = b.arrow([], b.block([]));
21-
22-
// Generate helper variables for clearer error messages
23-
const get = b.id(state.scope.generate(id.name + '_get'));
24-
const set = b.id(state.scope.generate(id.name + '_set'));
17+
const expression_text = dev
18+
? b.literal(source.slice(spread_expression.argument.start, spread_expression.argument.end))
19+
: undefined;
2520

26-
const getter = b.logical(
27-
'??',
28-
b.conditional(
29-
b.call('Array.isArray', id),
30-
b.member(id, b.literal(0), true),
31-
b.member(id, b.id('get'))
32-
),
33-
noop
21+
const id = state.scope.generate('$$spread_binding');
22+
const get = b.id(id + '_get');
23+
const set = b.id(id + '_set');
24+
state.init.push(
25+
b.const(
26+
b.array_pattern([get, set]),
27+
b.call('$.validate_spread_bindings', visited_expression, expression_text)
28+
)
3429
);
35-
const setter = b.logical(
36-
'??',
37-
b.conditional(
38-
b.call('Array.isArray', id),
39-
b.member(id, b.literal(1), true),
40-
b.member(id, b.id('set'))
41-
),
42-
noop
43-
);
44-
45-
state.init.push(b.const(get, getter));
46-
state.init.push(b.const(set, setter));
4730

4831
return { get, set };
4932
}

‎packages/svelte/src/compiler/utils/builders.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -649,13 +649,13 @@ function return_builder(argument = null) {
649649
}
650650

651651
/**
652-
* @param {string} str
652+
* @param {string | ESTree.TemplateLiteral} str
653653
* @returns {ESTree.ThrowStatement}
654654
*/
655655
export function throw_error(str) {
656656
return {
657657
type: 'ThrowStatement',
658-
argument: new_builder('Error', literal(str))
658+
argument: new_builder('Error', typeof str === 'string' ? literal(str) : str)
659659
};
660660
}
661661

‎packages/svelte/src/internal/client/index.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,8 @@ export {
172172
validate_dynamic_element_tag,
173173
validate_store,
174174
validate_void_dynamic_element,
175-
prevent_snippet_stringification
175+
prevent_snippet_stringification,
176+
validate_spread_bindings
176177
} from '../shared/validate.js';
177178
export { strict_equals, equals } from './dev/equality.js';
178179
export { log_if_contains_state } from './dev/console-log.js';

‎packages/svelte/src/internal/server/index.js‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -511,13 +511,14 @@ export { assign_payload, copy_payload } from './payload.js';
511511

512512
export { snapshot } from '../shared/clone.js';
513513

514-
export { fallback, to_array } from '../shared/utils.js';
514+
export { fallback, to_array, noop } from '../shared/utils.js';
515515

516516
export {
517517
invalid_default_snippet,
518518
validate_dynamic_element_tag,
519519
validate_void_dynamic_element,
520-
prevent_snippet_stringification
520+
prevent_snippet_stringification,
521+
validate_spread_bindings
521522
} from '../shared/validate.js';
522523

523524
export { escape_html as escape };

‎packages/svelte/src/internal/shared/errors.js‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,4 +114,21 @@ export function svelte_element_invalid_this_value() {
114114
} else {
115115
throw new Error(`https://svelte.dev/e/svelte_element_invalid_this_value`);
116116
}
117-
}
117+
}
118+
119+
/**
120+
* `%name%%member%` must be a function or `undefined`
121+
* @param {string} name
122+
* @returns {never}
123+
*/
124+
export function invalid_spread_bindings(name) {
125+
if (DEV) {
126+
const error = new Error(`invalid_spread_bindings\n\`${name}\` must be a function or \`undefined\`\nhttps://svelte.dev/e/invalid_spread_bindings`);
127+
128+
error.name = 'Svelte error';
129+
130+
throw error;
131+
} else {
132+
throw new Error(`https://svelte.dev/e/invalid_spread_bindings`);
133+
}
134+
}

0 commit comments

Comments
 (0)