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

breaking: deprecate context="module" in favor of module #12948

Merged
merged 7 commits into from
Aug 21, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/quick-eagles-sit.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

breaking: deprecate `context="module"` in favor of `module`
Original file line number Diff line number Diff line change
Expand Up @@ -161,16 +161,16 @@ If you'd like to react to changes to a prop, use the `$derived` or `$effect` run

For more information on reactivity, read the documentation around runes.

## <script context="module">
## <script module>

A `<script>` tag with a `context="module"` attribute runs once when the module first evaluates, rather than for each component instance. Values declared in this block are accessible from a regular `<script>` (and the component markup) but not vice versa.
A `<script>` tag with a `module` attribute runs once when the module first evaluates, rather than for each component instance. Values declared in this block are accessible from a regular `<script>` (and the component markup) but not vice versa.

You can `export` bindings from this block, and they will become exports of the compiled module.

You cannot `export default`, since the default export is the component itself.

```svelte
<script context="module">
<script module>
let totalComponents = 0;

// the export keyword allows this function to imported with e.g.
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
"playwright": "^1.41.1",
"prettier": "^3.2.4",
"prettier-plugin-svelte": "^3.1.2",
"svelte": "workspace:^",
"typescript": "^5.5.2",
"typescript-eslint": "^8.0.0-alpha.34",
"v8-natives": "^1.2.5",
Expand Down
4 changes: 2 additions & 2 deletions packages/svelte/messages/compile-errors/script.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

## declaration_duplicate_module_import

> Cannot declare same variable name which is imported inside `<script context="module">`
> Cannot declare a variable with the same name as an import inside `<script module>`

## derived_invalid_export

Expand Down Expand Up @@ -152,7 +152,7 @@

## store_invalid_subscription

> Cannot reference store value inside `<script context="module">`
> Cannot reference store value inside `<script module>`

## store_invalid_subscription_module

Expand Down
10 changes: 9 additions & 1 deletion packages/svelte/messages/compile-errors/template.md
Original file line number Diff line number Diff line change
Expand Up @@ -216,12 +216,20 @@ HTML restricts where certain elements can appear. In case of a violation the bro

## script_duplicate

> A component can have a single top-level `<script>` element and/or a single top-level `<script context="module">` element
> A component can have a single top-level `<script>` element and/or a single top-level `<script module>` element

## script_invalid_attribute_value

> If the `%name%` attribute is supplied, it must be a boolean attribute

## script_invalid_context

> If the context attribute is supplied, its value must be "module"

## script_reserved_attribute

> The `%name%` attribute is reserved and cannot be used

## slot_attribute_duplicate

> Duplicate slot name '%name%' in <%component%>
Expand Down
8 changes: 8 additions & 0 deletions packages/svelte/messages/compile-warnings/template.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,14 @@ HTML restricts where certain elements can appear. In case of a violation the bro

This code will work when the component is rendered on the client (which is why this is a warning rather than an error), but if you use server rendering it will cause hydration to fail.

## script_context_deprecated

> `context="module"` is deprecated, use the `module` attribute instead

## script_unknown_attribute

> Unrecognized attribute — should be one of `generics`, `lang` or `module`. If this exists for a preprocessor, ensure that the preprocessor removes it

## slot_element_deprecated

> Using `<slot>` to render parent content is deprecated. Use `{@render ...}` tags instead
Expand Down
32 changes: 26 additions & 6 deletions packages/svelte/src/compiler/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,12 +99,12 @@ export function declaration_duplicate(node, name) {
}

/**
* Cannot declare same variable name which is imported inside `<script context="module">`
* Cannot declare a variable with the same name as an import inside `<script module>`
* @param {null | number | NodeLike} node
* @returns {never}
*/
export function declaration_duplicate_module_import(node) {
e(node, "declaration_duplicate_module_import", "Cannot declare same variable name which is imported inside `<script context=\"module\">`");
e(node, "declaration_duplicate_module_import", "Cannot declare a variable with the same name as an import inside `<script module>`");
}

/**
Expand Down Expand Up @@ -417,12 +417,12 @@ export function store_invalid_scoped_subscription(node) {
}

/**
* Cannot reference store value inside `<script context="module">`
* Cannot reference store value inside `<script module>`
* @param {null | number | NodeLike} node
* @returns {never}
*/
export function store_invalid_subscription(node) {
e(node, "store_invalid_subscription", "Cannot reference store value inside `<script context=\"module\">`");
e(node, "store_invalid_subscription", "Cannot reference store value inside `<script module>`");
}

/**
Expand Down Expand Up @@ -1044,12 +1044,22 @@ export function render_tag_invalid_spread_argument(node) {
}

/**
* A component can have a single top-level `<script>` element and/or a single top-level `<script context="module">` element
* A component can have a single top-level `<script>` element and/or a single top-level `<script module>` element
* @param {null | number | NodeLike} node
* @returns {never}
*/
export function script_duplicate(node) {
e(node, "script_duplicate", "A component can have a single top-level `<script>` element and/or a single top-level `<script context=\"module\">` element");
e(node, "script_duplicate", "A component can have a single top-level `<script>` element and/or a single top-level `<script module>` element");
}

/**
* If the `%name%` attribute is supplied, it must be a boolean attribute
* @param {null | number | NodeLike} node
* @param {string} name
* @returns {never}
*/
export function script_invalid_attribute_value(node, name) {
e(node, "script_invalid_attribute_value", `If the \`${name}\` attribute is supplied, it must be a boolean attribute`);
}

/**
Expand All @@ -1061,6 +1071,16 @@ export function script_invalid_context(node) {
e(node, "script_invalid_context", "If the context attribute is supplied, its value must be \"module\"");
}

/**
* The `%name%` attribute is reserved and cannot be used
* @param {null | number | NodeLike} node
* @param {string} name
* @returns {never}
*/
export function script_reserved_attribute(node, name) {
e(node, "script_reserved_attribute", `The \`${name}\` attribute is reserved and cannot be used`);
}

/**
* Duplicate slot name '%name%' in <%component%>
* @param {null | number | NodeLike} node
Expand Down
14 changes: 10 additions & 4 deletions packages/svelte/src/compiler/migrate/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
/** @import { VariableDeclarator, Node, Identifier } from 'estree' */
/** @import { SvelteNode } from '../types/template.js' */
/** @import { Visitors } from 'zimmerframe' */
/** @import { ComponentAnalysis } from '../phases/types.js' */
/** @import { Scope } from '../phases/scope.js' */
Expand Down Expand Up @@ -58,6 +57,13 @@ export function migrate(source) {
needs_run: false
};

if (parsed.module) {
const context = parsed.module.attributes.find((attr) => attr.name === 'context');
if (context) {
state.str.update(context.start, context.end, 'module');
}
}

if (parsed.instance) {
walk(parsed.instance.content, state, instance_script);
}
Expand Down Expand Up @@ -223,7 +229,7 @@ export function migrate(source) {
* }} State
*/

/** @type {Visitors<SvelteNode, State>} */
/** @type {Visitors<Compiler.SvelteNode, State>} */
const instance_script = {
_(node, { state, next }) {
// @ts-expect-error
Expand Down Expand Up @@ -472,7 +478,7 @@ const instance_script = {
}
};

/** @type {Visitors<SvelteNode, State>} */
/** @type {Visitors<Compiler.SvelteNode, State>} */
const template = {
Identifier(node, { state, path }) {
handle_identifier(node, state, path);
Expand Down Expand Up @@ -590,7 +596,7 @@ const template = {
/**
* @param {VariableDeclarator} declarator
* @param {MagicString} str
* @param {Compiler.SvelteNode[]} path
* @param {Array<Compiler.SvelteNode>} path
*/
function extract_type_and_comment(declarator, str, path) {
const parent = path.at(-1);
Expand Down
72 changes: 48 additions & 24 deletions packages/svelte/src/compiler/phases/1-parse/read/script.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,32 +4,14 @@
import * as acorn from '../acorn.js';
import { regex_not_newline_characters } from '../../patterns.js';
import * as e from '../../../errors.js';
import * as w from '../../../warnings.js';
import { is_text_attribute } from '../../../utils/ast.js';

const regex_closing_script_tag = /<\/script\s*>/;
const regex_starts_with_closing_script_tag = /^<\/script\s*>/;

/**
* @param {any[]} attributes
* @returns {string}
*/
function get_context(attributes) {
const context = attributes.find(
/** @param {any} attribute */ (attribute) => attribute.name === 'context'
);
if (!context) return 'default';

if (context.value.length !== 1 || context.value[0].type !== 'Text') {
e.script_invalid_context(context.start);
}

const value = context.value[0].data;

if (value !== 'module') {
e.script_invalid_context(context.start);
}

return value;
}
const RESERVED_ATTRIBUTES = ['server', 'client', 'worker', 'test', 'default'];
const ALLOWED_ATTRIBUTES = ['context', 'generics', 'lang', 'module'];

/**
* @param {Parser} parser
Expand Down Expand Up @@ -60,14 +42,56 @@ export function read_script(parser, start, attributes) {
// TODO is this necessary?
ast.start = script_start;

/** @type {'default' | 'module'} */
let context = 'default';

for (const attribute of /** @type {Attribute[]} */ (attributes)) {
if (RESERVED_ATTRIBUTES.includes(attribute.name)) {
e.script_reserved_attribute(attribute, attribute.name);
}

if (!ALLOWED_ATTRIBUTES.includes(attribute.name)) {
w.script_unknown_attribute(attribute);
}

if (attribute.name === 'module') {
if (attribute.value !== true) {
// Deliberately a generic code to future-proof for potential other attributes
e.script_invalid_attribute_value(attribute, attribute.name);
}

context = 'module';
}

if (attribute.name === 'context') {
if (attribute.value === true || !is_text_attribute(attribute)) {
throw new Error('TODO');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wasn't this supposed to be e.script_invalid_context(attribute);?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes! good spot, thanks — #12973

}

if (attribute.value.length !== 1 || attribute.value[0].type !== 'Text') {
e.script_invalid_context(attribute);
}

const value = attribute.value[0].data;

if (value !== 'module') {
e.script_invalid_context(attribute);
}

w.script_context_deprecated(attribute);

context = 'module';
}
}

return {
type: 'Script',
start,
end: parser.index,
context: get_context(attributes),
context,
content: ast,
parent: null,
// @ts-ignore
attributes: attributes
attributes
};
}
2 changes: 1 addition & 1 deletion packages/svelte/src/compiler/phases/2-analyze/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ export function analyze_component(root, source, options) {

if (module.ast) {
for (const { node, path } of references) {
// if the reference is inside context="module", error. this is a bit hacky but it works
// if the reference is inside module, error. this is a bit hacky but it works
if (
/** @type {number} */ (node.start) > /** @type {number} */ (module.ast.start) &&
/** @type {number} */ (node.end) < /** @type {number} */ (module.ast.end) &&
Expand Down
4 changes: 2 additions & 2 deletions packages/svelte/src/compiler/types/template.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export interface Root extends BaseNode {
css: Css.StyleSheet | null;
/** The parsed `<script>` element, if exists */
instance: Script | null;
/** The parsed `<script context="module">` element, if exists */
/** The parsed `<script module>` element, if exists */
module: Script | null;
metadata: {
/** Whether the component was parsed with typescript */
Expand Down Expand Up @@ -488,7 +488,7 @@ export type SvelteNode = Node | TemplateNode | Fragment | Css.Node;

export interface Script extends BaseNode {
type: 'Script';
context: string;
context: 'default' | 'module';
content: Program;
attributes: Attribute[];
}
Expand Down
18 changes: 18 additions & 0 deletions packages/svelte/src/compiler/warnings.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,8 @@ export const codes = [
"element_invalid_self_closing_tag",
"event_directive_deprecated",
"node_invalid_placement_ssr",
"script_context_deprecated",
"script_unknown_attribute",
"slot_element_deprecated",
"svelte_component_deprecated",
"svelte_element_invalid_this"
Expand Down Expand Up @@ -769,6 +771,22 @@ export function node_invalid_placement_ssr(node, thing, parent) {
w(node, "node_invalid_placement_ssr", `${thing} is invalid inside \`<${parent}>\`. When rendering this component on the server, the resulting HTML will be modified by the browser, likely resulting in a \`hydration_mismatch\` warning`);
}

/**
* `context="module"` is deprecated, use the `module` attribute instead
* @param {null | NodeLike} node
*/
export function script_context_deprecated(node) {
w(node, "script_context_deprecated", "`context=\"module\"` is deprecated, use the `module` attribute instead");
}

/**
* Unrecognized attribute — should be one of `generics`, `lang` or `module`. If this exists for a preprocessor, ensure that the preprocessor removes it
* @param {null | NodeLike} node
*/
export function script_unknown_attribute(node) {
w(node, "script_unknown_attribute", "Unrecognized attribute — should be one of `generics`, `lang` or `module`. If this exists for a preprocessor, ensure that the preprocessor removes it");
}

/**
* Using `<slot>` to render parent content is deprecated. Use `{@render ...}` tags instead
* @param {null | NodeLike} node
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@ export default test({
code: 'state_invalid_export',
message:
"Cannot export state from a module if it is reassigned. Either export a function returning the state value or only mutate the state value's properties",
position: [76, 114]
position: [66, 104]
}
});
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<script context="module">
<script module>
export const object = $state({
ok: true
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<script context="module">
<script module>
export let bar = ''; // check that it doesn't error here already
</script>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { test } from '../../test';
export default test({
error: {
code: 'store_invalid_subscription',
message: 'Cannot reference store value inside `<script context="module">`',
position: [164, 168]
message: 'Cannot reference store value inside `<script module>`',
position: [154, 158]
}
});
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<script context="module">
<script module>
// this should be fine (state rune is not treated as a store)
const state = $state(0);
// this is not
Expand Down
Loading
Loading