Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: implement message overloads #11318

Merged
merged 1 commit into from
Apr 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 7 additions & 1 deletion packages/svelte/messages/client-warnings/warnings.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,10 @@

## ownership_invalid_binding

> %parent% passed a value to %child% with `bind:`, but the value is owned by %owner%. Consider creating a binding between %owner% and %parent%
> %parent% passed a value to %child% with `bind:`, but the value is owned by %owner%. Consider creating a binding between %owner% and %parent%

## ownership_invalid_mutation

> Mutating a value outside the component that created it is strongly discouraged. Consider passing values to child components with `bind:`, or use a callback instead

> %component% mutated a value owned by %owner%. This is strongly discouraged. Consider passing values to child components with `bind:`, or use a callback instead
4 changes: 1 addition & 3 deletions packages/svelte/messages/compile-errors/bindings.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@

> `bind:%name%` is not a valid binding

## bind_invalid_detailed

> `bind:%name%` is not a valid binding. %explanation%

## invalid_type_attribute
Expand All @@ -32,4 +30,4 @@

## dynamic_contenteditable_attribute

> 'contenteditable' attribute cannot be dynamic if element uses two-way binding
> 'contenteditable' attribute cannot be dynamic if element uses two-way binding
6 changes: 1 addition & 5 deletions packages/svelte/messages/compile-warnings/a11y.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@

> Unknown aria attribute 'aria-%attribute%'

## a11y_unknown_aria_attribute_suggestion

> Unknown aria attribute 'aria-%attribute%'. Did you mean '%suggestion%'?

## a11y_hidden
Expand Down Expand Up @@ -62,8 +60,6 @@

> Unknown role '%role%'

## a11y_unknown_role_suggestion

> Unknown role '%role%'. Did you mean '%suggestion%'?

## a11y_no_redundant_roles
Expand Down Expand Up @@ -168,4 +164,4 @@

## a11y_missing_content

> <%name%> element should have child content
> <%name%> element should have child content
4 changes: 2 additions & 2 deletions packages/svelte/messages/compile-warnings/options.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
## options_deprecated_accessors

The `accessors` option has been deprecated. It will have no effect in runes mode
> The `accessors` option has been deprecated. It will have no effect in runes mode

## options_deprecated_immutable

Expand All @@ -24,4 +24,4 @@ The `accessors` option has been deprecated. It will have no effect in runes mode

## options_removed_loop_guard_timeout

> The `loopGuardTimeout` option has been removed
> The `loopGuardTimeout` option has been removed
157 changes: 107 additions & 50 deletions packages/svelte/scripts/process-messages/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import * as acorn from 'acorn';
import { walk } from 'zimmerframe';
import * as esrap from 'esrap';

/** @type {Record<string, Record<string, { messages: string[], details: string | null }>>} */
const messages = {};
const seen = new Set();

Expand All @@ -24,12 +25,21 @@ for (const category of fs.readdirSync('messages')) {
throw new Error(`Duplicate message code ${category}/${code}`);
}

const sections = text.trim().split('\n\n');
let details = null;
if (!sections[sections.length - 1].startsWith('> ')) {
details = /** @type {string} */ (sections.pop());
}

if (sections.length === 0) {
throw new Error('No message text');
}

seen.add(code);
messages[category][code] = text
.trim()
.split('\n')
.map((line) => line.slice(2))
.join('\n');
messages[category][code] = {
messages: sections.map((section) => section.replace(/^> /gm, '')),
details
};
}
}
}
Expand Down Expand Up @@ -102,13 +112,89 @@ function transform(name, dest) {
ast.body.splice(index, 1);

for (const code in category) {
const message = category[code];
const { messages } = category[code];
const vars = [];
for (const match of message.matchAll(/%(\w+)%/g)) {
const name = match[1];
if (!vars.includes(name)) {
vars.push(match[1]);

const group = messages.map((text, i) => {
for (const match of text.matchAll(/%(\w+)%/g)) {
const name = match[1];
if (!vars.includes(name)) {
vars.push(match[1]);
}
}

return {
text,
vars: vars.slice()
};
});

/** @type {import('estree').Expression} */
let message = { type: 'Literal', value: '' };
let prev_vars;

for (let i = 0; i < group.length; i += 1) {
const { text, vars } = group[i];

if (vars.length === 0) {
message = {
type: 'Literal',
value: text
};
continue;
}

const parts = text.split(/(%\w+%)/);

/** @type {import('estree').Expression[]} */
const expressions = [];

/** @type {import('estree').TemplateElement[]} */
const quasis = [];

for (let i = 0; i < parts.length; i += 1) {
const part = parts[i];
if (i % 2 === 0) {
const str = part.replace(/(`|\${)/g, '\\$1');
quasis.push({
type: 'TemplateElement',
value: { raw: str, cooked: str },
tail: i === parts.length - 1
});
} else {
expressions.push({
type: 'Identifier',
name: part.slice(1, -1)
});
}
}

/** @type {import('estree').Expression} */
const expression = {
type: 'TemplateLiteral',
expressions,
quasis
};

if (prev_vars) {
if (vars.length === prev_vars.length) {
throw new Error('Message overloads must have new parameters');
}

message = {
type: 'ConditionalExpression',
test: {
type: 'Identifier',
name: vars[prev_vars.length]
},
consequent: expression,
alternate: message
};
} else {
message = expression;
}

prev_vars = vars;
}

const clone = walk(/** @type {import('estree').Node} */ (template_node), null, {
Expand All @@ -120,14 +206,22 @@ function transform(name, dest) {
.split('\n')
.map((line) => {
if (line === ' * MESSAGE') {
return message
return messages[messages.length - 1]
.split('\n')
.map((line) => ` * ${line}`)
.join('\n');
}

if (line.includes('PARAMETER')) {
return vars.map((name) => ` * @param {string} ${name}`).join('\n');
return vars
.map((name, i) => {
const optional = i >= group[0].vars.length;

return optional
? ` * @param {string | undefined | null} [${name}]`
: ` * @param {string} ${name}`;
})
.join('\n');
}

return line;
Expand Down Expand Up @@ -171,44 +265,7 @@ function transform(name, dest) {
},
Identifier(node) {
if (node.name !== 'MESSAGE') return;

if (/%\w+%/.test(message)) {
const parts = message.split(/(%\w+%)/);

/** @type {import('estree').Expression[]} */
const expressions = [];

/** @type {import('estree').TemplateElement[]} */
const quasis = [];

for (let i = 0; i < parts.length; i += 1) {
const part = parts[i];
if (i % 2 === 0) {
const str = part.replace(/(`|\${)/g, '\\$1');
quasis.push({
type: 'TemplateElement',
value: { raw: str, cooked: str },
tail: i === parts.length - 1
});
} else {
expressions.push({
type: 'Identifier',
name: part.slice(1, -1)
});
}
}

return {
type: 'TemplateLiteral',
expressions,
quasis
};
}

return {
type: 'Literal',
value: message
};
return message;
}
});

Expand Down
16 changes: 3 additions & 13 deletions packages/svelte/src/compiler/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -227,25 +227,15 @@ export function bind_invalid_target(node, name, elements) {
e(node, "bind_invalid_target", `\`bind:${name}\` can only be used with ${elements}`);
}

/**
* `bind:%name%` is not a valid binding
* @param {null | number | NodeLike} node
* @param {string} name
* @returns {never}
*/
export function bind_invalid(node, name) {
e(node, "bind_invalid", `\`bind:${name}\` is not a valid binding`);
}

/**
* `bind:%name%` is not a valid binding. %explanation%
* @param {null | number | NodeLike} node
* @param {string} name
* @param {string} explanation
* @param {string | undefined | null} [explanation]
* @returns {never}
*/
export function bind_invalid_detailed(node, name, explanation) {
e(node, "bind_invalid_detailed", `\`bind:${name}\` is not a valid binding. ${explanation}`);
export function bind_invalid(node, name, explanation) {
e(node, "bind_invalid", explanation ? `\`bind:${name}\` is not a valid binding. ${explanation}` : `\`bind:${name}\` is not a valid binding`);
}

/**
Expand Down
13 changes: 2 additions & 11 deletions packages/svelte/src/compiler/phases/2-analyze/a11y.js
Original file line number Diff line number Diff line change
Expand Up @@ -739,12 +739,7 @@ function check_element(node, state) {
const type = name.slice(5);
if (!aria_attributes.includes(type)) {
const match = fuzzymatch(type, aria_attributes);
if (match) {
// TODO allow 'overloads' in messages, so that we can use the same code with and without suggestions
w.a11y_unknown_aria_attribute_suggestion(attribute, type, match);
} else {
w.a11y_unknown_aria_attribute(attribute, type);
}
w.a11y_unknown_aria_attribute(attribute, type, match);
}

if (name === 'aria-hidden' && regex_heading_tags.test(node.name)) {
Expand Down Expand Up @@ -792,11 +787,7 @@ function check_element(node, state) {
w.a11y_no_abstract_role(attribute, current_role);
} else if (current_role && !aria_roles.includes(current_role)) {
const match = fuzzymatch(current_role, aria_roles);
if (match) {
w.a11y_unknown_role_suggestion(attribute, current_role, match);
} else {
w.a11y_unknown_role(attribute, current_role);
}
w.a11y_unknown_role(attribute, current_role, match);
}

// no-redundant-roles
Expand Down
4 changes: 2 additions & 2 deletions packages/svelte/src/compiler/phases/2-analyze/validation.js
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ const validation = {
parent?.type === 'SvelteBody'
) {
if (context.state.options.namespace === 'foreign' && node.name !== 'this') {
e.bind_invalid_detailed(node, node.name, 'Foreign elements only support `bind:this`');
e.bind_invalid(node, node.name, 'Foreign elements only support `bind:this`');
}

if (node.name in binding_properties) {
Expand Down Expand Up @@ -442,7 +442,7 @@ const validation = {
if (match) {
const property = binding_properties[match];
if (!property.valid_elements || property.valid_elements.includes(parent.name)) {
e.bind_invalid_detailed(node, node.name, `Did you mean '${match}'?`);
e.bind_invalid(node, node.name, `Did you mean '${match}'?`);
}
}
e.bind_invalid(node, node.name);
Expand Down