Skip to content

Commit

Permalink
breaking: warn/error on old syntax in runes mode (#11203)
Browse files Browse the repository at this point in the history
* breaking: warn/error on old syntax in runes mode

- warn on slots and event handlers in runes mode
- error on `<slot>` + `{@render ...}` tag usage in same component

closes #9416

* render tag + slot could occur in legacy mode as well, error there, too
  • Loading branch information
dummdidumm authored Apr 17, 2024
1 parent 4ef6454 commit d51075c
Show file tree
Hide file tree
Showing 11 changed files with 106 additions and 6 deletions.
5 changes: 5 additions & 0 deletions .changeset/four-pugs-listen.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"svelte": patch
---

breaking: warn on slots and event handlers in runes mode, error on `<slot>` + `{@render ...}` tag usage in same component
4 changes: 3 additions & 1 deletion packages/svelte/src/compiler/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,9 @@ const special_elements = {
* @param {string | null} match
*/
'invalid-svelte-tag': (tags, match) =>
`Valid <svelte:...> tag names are ${list(tags)}${match ? ' (did you mean ' + match + '?)' : ''}`
`Valid <svelte:...> tag names are ${list(tags)}${match ? ' (did you mean ' + match + '?)' : ''}`,
'conflicting-slot-usage': () =>
`Cannot use <slot> syntax and {@render ...} tags in the same component. Migrate towards {@render ...} tags completely.`
};

/** @satisfies {Errors} */
Expand Down
9 changes: 7 additions & 2 deletions packages/svelte/src/compiler/phases/2-analyze/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,7 @@ export function analyze_component(root, source, options) {
uses_rest_props: false,
uses_slots: false,
uses_component_bindings: false,
uses_render_tags: false,
custom_element: options.customElementOptions ?? options.customElement,
inject_styles: options.css === 'injected' || options.customElement,
accessors: options.customElement
Expand All @@ -388,7 +389,7 @@ export function analyze_component(root, source, options) {
!!options.legacy?.componentApi,
reactive_statements: new Map(),
binding_groups: new Map(),
slot_names: new Set(),
slot_names: new Map(),
warnings,
css: {
ast: root.css,
Expand Down Expand Up @@ -502,6 +503,10 @@ export function analyze_component(root, source, options) {
analysis.reactive_statements = order_reactive_statements(analysis.reactive_statements);
}

if (analysis.uses_render_tags && (analysis.uses_slots || analysis.slot_names.size > 0)) {
error(analysis.slot_names.values().next().value, 'conflicting-slot-usage');
}

// warn on any nonstate declarations that are a) reassigned and b) referenced in the template
for (const scope of [module.scope, instance.scope]) {
outer: for (const [name, binding] of scope.declarations) {
Expand Down Expand Up @@ -1087,7 +1092,7 @@ const common_visitors = {
break;
}
}
context.state.analysis.slot_names.add(name);
context.state.analysis.slot_names.set(name, node);
},
StyleDirective(node, context) {
if (node.value === true) {
Expand Down
14 changes: 14 additions & 0 deletions packages/svelte/src/compiler/phases/2-analyze/validation.js
Original file line number Diff line number Diff line change
Expand Up @@ -578,6 +578,8 @@ const validation = {
});
},
RenderTag(node, context) {
context.state.analysis.uses_render_tags = true;

const raw_args = unwrap_optional(node.expression).arguments;
for (const arg of raw_args) {
if (arg.type === 'SpreadElement') {
Expand Down Expand Up @@ -1183,6 +1185,18 @@ export const validation_runes = merge(validation, a11y_validators, {
warn(state.analysis.warnings, node, path, 'invalid-bindable-declaration');
}
},
SlotElement(node, { state, path }) {
if (!state.analysis.custom_element) {
warn(state.analysis.warnings, node, path, 'deprecated-slot-element');
}
},
OnDirective(node, { state, path }) {
const parent_type = path.at(-1)?.type;
// Don't warn on component events; these might not be under the author's control so the warning would be unactionable
if (parent_type === 'RegularElement' || parent_type === 'SvelteElement') {
warn(state.analysis.warnings, node, path, 'deprecated-event-handler', node.name);
}
},
// TODO this is a code smell. need to refactor this stuff
ClassBody: validation_runes_js.ClassBody,
ClassDeclaration: validation_runes_js.ClassDeclaration,
Expand Down
4 changes: 3 additions & 1 deletion packages/svelte/src/compiler/phases/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type {
Css,
Fragment,
RegularElement,
SlotElement,
SvelteElement,
SvelteNode,
SvelteOptions
Expand Down Expand Up @@ -61,13 +62,14 @@ export interface ComponentAnalysis extends Analysis {
/** Whether the component uses `$$slots` */
uses_slots: boolean;
uses_component_bindings: boolean;
uses_render_tags: boolean;
custom_element: boolean | SvelteOptions['customElement'];
/** If `true`, should append styles through JavaScript */
inject_styles: boolean;
reactive_statements: Map<LabeledStatement, ReactiveStatement>;
/** Identifiers that make up the `bind:group` expression -> internal group binding name */
binding_groups: Map<[key: string, bindings: Array<Binding | null>], Identifier>;
slot_names: Set<string>;
slot_names: Map<string, SlotElement>;
css: {
ast: Css.StyleSheet | null;
hash: string;
Expand Down
7 changes: 6 additions & 1 deletion packages/svelte/src/compiler/warnings.js
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,12 @@ const legacy = {
'All dependencies of the reactive declaration are declared in a module script and will not be reactive',
/** @param {string} name */
'unused-export-let': (name) =>
`Component has unused export property '${name}'. If it is for external reference only, please consider using \`export const ${name}\``
`Component has unused export property '${name}'. If it is for external reference only, please consider using \`export const ${name}\``,
'deprecated-slot-element': () =>
`Using <slot> to render parent content is deprecated. Use {@render ...} tags instead.`,
/** @param {string} name */
'deprecated-event-handler': (name) =>
`Using on:${name} to listen to the ${name} event is is deprecated. Use the event attribute on${name} instead.`
};

const block = {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { test } from '../../test';

export default test({
error: {
code: 'conflicting-slot-usage',
message:
'Cannot use <slot> syntax and {@render ...} tags in the same component. Migrate towards {@render ...} tags completely.',
position: [71, 84]
}
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<script>
let { children } = $props();
</script>

{@render children()}
<slot></slot>
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<script>
let { foo } = $props();
</script>

<!-- ok -->
<button onclick={foo}>click me</button>
<Button onclick={foo}>click me</Button>
<Button on:click={foo}>click me</Button>

<!-- warn -->
<slot></slot>
<slot name="foo"></slot>
<button on:click={foo}>click me</button>
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
[
{
"code": "deprecated-slot-element",
"end": {
"column": 13,
"line": 11
},
"message": "Using <slot> to render parent content is deprecated. Use {@render ...} tags instead.",
"start": {
"column": 0,
"line": 11
}
},
{
"code": "deprecated-slot-element",
"end": {
"column": 24,
"line": 12
},
"message": "Using <slot> to render parent content is deprecated. Use {@render ...} tags instead.",
"start": {
"column": 0,
"line": 12
}
},
{
"code": "deprecated-event-handler",
"end": {
"column": 22,
"line": 13
},
"message": "Using on:click to listen to the click event is is deprecated. Use the event attribute onclick instead.",
"start": {
"column": 8,
"line": 13
}
}
]
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@
console.log(doubled);
</script>

<button on:click={() => count += 1}>
<button onclick={() => count += 1}>
clicks: {count}
</button>

0 comments on commit d51075c

Please sign in to comment.