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

feat: implement message overloads (#11318)

Browse Source
pull/11330/head
Rich Harris 3 months ago committed by GitHub
parent 4be593472d
commit 6ad5cd4461
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
19 changed files with 163 additions and 187 deletions
Show Stats Download Patch File Download Diff File

8
packages/svelte/messages/client-warnings/warnings.md
Unescape View File

@ -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
packages/svelte/messages/compile-errors/bindings.md
Unescape View File

@ -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
@ -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
packages/svelte/messages/compile-warnings/a11y.md
Unescape View File

@ -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
@ -62,8 +60,6 @@
> Unknown role '%role%'
## a11y_unknown_role_suggestion
> Unknown role '%role%'. Did you mean '%suggestion%'?
## a11y_no_redundant_roles
@ -168,4 +164,4 @@
## a11y_missing_content
> <%name%> element should have child content
> <%name%> element should have child content

4
packages/svelte/messages/compile-warnings/options.md
Unescape View File

@ -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
@ -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
packages/svelte/scripts/process-messages/index.js
Unescape View File

@ -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();
@ -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
};
}
}
}
@ -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, {
@ -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;
@ -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;
}
});

16
packages/svelte/src/compiler/errors.js
Unescape View File

@ -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`);
}
/**

13
packages/svelte/src/compiler/phases/2-analyze/a11y.js
Unescape View File

@ -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)) {
@ -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

4
packages/svelte/src/compiler/phases/2-analyze/validation.js
Unescape View File

@ -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) {
@ -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);

34
packages/svelte/src/compiler/warnings.js
Unescape View File

@ -49,23 +49,14 @@ export function a11y_aria_attributes(node, name) {
w(node, "a11y_aria_attributes", `<${name}> should not have aria-* attributes`);
}
/**
* Unknown aria attribute 'aria-%attribute%'
* @param {null | NodeLike} node
* @param {string} attribute
*/
export function a11y_unknown_aria_attribute(node, attribute) {
w(node, "a11y_unknown_aria_attribute", `Unknown aria attribute 'aria-${attribute}'`);
}
/**
* Unknown aria attribute 'aria-%attribute%'. Did you mean '%suggestion%'?
* @param {null | NodeLike} node
* @param {string} attribute
* @param {string} suggestion
* @param {string | undefined | null} [suggestion]
*/
export function a11y_unknown_aria_attribute_suggestion(node, attribute, suggestion) {
w(node, "a11y_unknown_aria_attribute_suggestion", `Unknown aria attribute 'aria-${attribute}'. Did you mean '${suggestion}'?`);
export function a11y_unknown_aria_attribute(node, attribute, suggestion) {
w(node, "a11y_unknown_aria_attribute", suggestion ? `Unknown aria attribute 'aria-${attribute}'. Did you mean '${suggestion}'?` : `Unknown aria attribute 'aria-${attribute}'`);
}
/**
@ -178,23 +169,14 @@ export function a11y_no_abstract_role(node, role) {
w(node, "a11y_no_abstract_role", `Abstract role '${role}' is forbidden`);
}
/**
* Unknown role '%role%'
* @param {null | NodeLike} node
* @param {string} role
*/
export function a11y_unknown_role(node, role) {
w(node, "a11y_unknown_role", `Unknown role '${role}'`);
}
/**
* Unknown role '%role%'. Did you mean '%suggestion%'?
* @param {null | NodeLike} node
* @param {string} role
* @param {string} suggestion
* @param {string | undefined | null} [suggestion]
*/
export function a11y_unknown_role_suggestion(node, role, suggestion) {
w(node, "a11y_unknown_role_suggestion", `Unknown role '${role}'. Did you mean '${suggestion}'?`);
export function a11y_unknown_role(node, role, suggestion) {
w(node, "a11y_unknown_role", suggestion ? `Unknown role '${role}'. Did you mean '${suggestion}'?` : `Unknown role '${role}'`);
}
/**
@ -545,11 +527,11 @@ export function invalid_self_closing_tag(node, name) {
}
/**
* e `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
* @param {null | NodeLike} node
*/
export function options_deprecated_accessors(node) {
w(node, "options_deprecated_accessors", "e `accessors` option has been deprecated. It will have no effect in runes mode");
w(node, "options_deprecated_accessors", "The `accessors` option has been deprecated. It will have no effect in runes mode");
}
/**

17
packages/svelte/src/internal/client/dev/ownership.js
Unescape View File

@ -225,18 +225,13 @@ export function check_ownership(metadata) {
if (component && !has_owner(metadata, component)) {
let original = get_owner(metadata);
let message =
// @ts-expect-error
if (original.filename !== component.filename) {
// @ts-expect-error
original.filename !== component.filename
? // @ts-expect-error
`${component.filename} mutated a value owned by ${original.filename}. This is strongly discouraged`
: 'Mutating a value outside the component that created it is strongly discouraged';
// TODO get rid of this, but implement message overloads first
// eslint-disable-next-line no-console
console.warn(
`${message}. Consider passing values to child components with \`bind:\`, or use a callback instead.`
);
w.ownership_invalid_mutation(component.filename, original.filename);
} else {
w.ownership_invalid_mutation();
}
// eslint-disable-next-line no-console
console.trace();

14
packages/svelte/src/internal/client/warnings.js
Unescape View File

@ -30,4 +30,18 @@ export function ownership_invalid_binding(parent, child, owner) {
// TODO print a link to the documentation
console.warn("ownership_invalid_binding");
}
}
/**
* %component% mutated a value owned by %owner%. This is strongly discouraged. Consider passing values to child components with `bind:`, or use a callback instead
* @param {string | undefined | null} [component]
* @param {string | undefined | null} [owner]
*/
export function ownership_invalid_mutation(component, owner) {
if (DEV) {
console.warn(`%c[svelte] ${"ownership_invalid_mutation"}\n%c${`${component} mutated a value owned by ${owner}. This is strongly discouraged. Consider passing values to child components with \`bind:\`, or use a callback instead`}`, bold, normal);
} else {
// TODO print a link to the documentation
console.warn("ownership_invalid_mutation");
}
}

19
packages/svelte/tests/runtime-runes/samples/non-local-mutation-discouraged/_config.js
Unescape View File

@ -1,14 +1,8 @@
import { test } from '../../test';
/** @type {typeof console.warn} */
let warn;
/** @type {typeof console.trace} */
let trace;
/** @type {any[]} */
let warnings = [];
export default test({
html: `<button>clicks: 0</button>`,
@ -17,31 +11,22 @@ export default test({
},
before_test: () => {
warn = console.warn;
trace = console.trace;
console.warn = (...args) => {
warnings.push(...args);
};
console.trace = () => {};
},
after_test: () => {
console.warn = warn;
console.trace = trace;
warnings = [];
},
async test({ assert, target }) {
async test({ assert, target, warnings }) {
const btn = target.querySelector('button');
await btn?.click();
assert.htmlEqual(target.innerHTML, `<button>clicks: 1</button>`);
assert.deepEqual(warnings, [
'.../samples/non-local-mutation-discouraged/Counter.svelte mutated a value owned by .../samples/non-local-mutation-discouraged/main.svelte. This is strongly discouraged. Consider passing values to child components with `bind:`, or use a callback instead.'
'.../samples/non-local-mutation-discouraged/Counter.svelte mutated a value owned by .../samples/non-local-mutation-discouraged/main.svelte. This is strongly discouraged. Consider passing values to child components with `bind:`, or use a callback instead'
]);
}
});

21
packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-5/_config.js
Unescape View File

@ -1,31 +1,12 @@
import { tick } from 'svelte';
import { test } from '../../test';
/** @type {typeof console.warn} */
let warn;
/** @type {any[]} */
let warnings = [];
export default test({
compileOptions: {
dev: true
},
before_test: () => {
warn = console.warn;
console.warn = (...args) => {
warnings.push(...args);
};
},
after_test: () => {
console.warn = warn;
warnings = [];
},
async test({ assert, target }) {
async test({ assert, target, warnings }) {
const [btn1, btn2] = target.querySelectorAll('button');
await btn1.click();

21
packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-7/_config.js
Unescape View File

@ -1,31 +1,12 @@
import { tick } from 'svelte';
import { test } from '../../test';
/** @type {typeof console.warn} */
let warn;
/** @type {any[]} */
let warnings = [];
export default test({
compileOptions: {
dev: true
},
before_test: () => {
warn = console.warn;
console.warn = (...args) => {
warnings.push(...args);
};
},
after_test: () => {
console.warn = warn;
warnings = [];
},
async test({ assert, target }) {
async test({ assert, target, warnings }) {
const [btn1, btn2] = target.querySelectorAll('button');
btn1.click();

2
packages/svelte/tests/runtime-runes/samples/non-local-mutation-with-binding-3/_config.js
Unescape View File

@ -33,7 +33,7 @@ export default test({
assert.htmlEqual(target.innerHTML, `<button>clicks: 1</button><button>clicks: 1</button>`);
assert.deepEqual(warnings, [
'.../samples/non-local-mutation-with-binding-3/Counter.svelte mutated a value owned by .../samples/non-local-mutation-with-binding-3/main.svelte. This is strongly discouraged. Consider passing values to child components with `bind:`, or use a callback instead.'
'.../samples/non-local-mutation-with-binding-3/Counter.svelte mutated a value owned by .../samples/non-local-mutation-with-binding-3/main.svelte. This is strongly discouraged. Consider passing values to child components with `bind:`, or use a callback instead'
]);
}
});

2
packages/svelte/tests/validator/samples/a11y-aria-props/warnings.json
Unescape View File

@ -1,6 +1,6 @@
[
{
"code": "a11y_unknown_aria_attribute_suggestion",
"code": "a11y_unknown_aria_attribute",
"message": "Unknown aria attribute 'aria-labeledby'. Did you mean 'labelledby'?",
"start": {
"line": 1,

4
packages/svelte/tests/validator/samples/a11y-aria-role/warnings.json
Unescape View File

@ -1,6 +1,6 @@
[
{
"code": "a11y_unknown_role_suggestion",
"code": "a11y_unknown_role",
"message": "Unknown role 'toooltip'. Did you mean 'tooltip'?",
"start": {
"line": 6,
@ -12,7 +12,7 @@
}
},
{
"code": "a11y_unknown_role_suggestion",
"code": "a11y_unknown_role",
"message": "Unknown role 'toooltip'. Did you mean 'tooltip'?",
"start": {
"line": 7,

2
packages/svelte/tests/validator/samples/binding-invalid-foreign-namespace/errors.json
Unescape View File

@ -1,6 +1,6 @@
[
{
"code": "bind_invalid_detailed",
"code": "bind_invalid",
"message": "`bind:value` is not a valid binding. Foreign elements only support `bind:this`",
"start": {
"line": 6,

2
packages/svelte/tests/validator/samples/window-binding-invalid-innerwidth/errors.json
Unescape View File

@ -1,6 +1,6 @@
[
{
"code": "bind_invalid_detailed",
"code": "bind_invalid",
"message": "`bind:innerwidth` is not a valid binding. Did you mean 'innerWidth'?",
"start": {
"line": 5,

Write Preview
Loading…
Cancel
Save