Skip to content

Commit

Permalink
feat: implement message overloads (#11318)
Browse files Browse the repository at this point in the history
  • Loading branch information
Rich-Harris committed Apr 24, 2024
1 parent 4be5934 commit 6ad5cd4
Show file tree
Hide file tree
Showing 19 changed files with 163 additions and 187 deletions.
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

0 comments on commit 6ad5cd4

Please sign in to comment.