fix: mark accessors and immutable as deprecated (#11277)

Rich-Harris · dummdidumm · web-flow · commit 476f2172b6bc · 2024-04-24T10:55:39.000-04:00
* fix: mark `accessors` and `immutable` as deprecated * add warnings for deprecated <svelte:options> attributes * disable accessors in runes mode * update tests * tidy up * the hell? * regenerate types * if I would get a dollar for every windows bug I fix I would be a millionaire by now * return instance _and_ props in runes mode, move flushSync into shared code, don't set accessors in runes mode * goddammit * note breaking change * fix * regenerate messages * Revert "return instance _and_ props in runes mode, move flushSync into shared code, don't set accessors in runes mode" This reverts commit a47827e. * pass instance to tests --------- Co-authored-by: Simon Holthausen <simon.holthausen@vercel.com>
diff --git a/.changeset/tiny-meals-deliver.md b/.changeset/tiny-meals-deliver.md
@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: mark `accessors` and `immutable` as deprecated
diff --git a/packages/svelte/messages/compile-warnings/legacy.md b/packages/svelte/messages/compile-warnings/legacy.md
@@ -16,4 +16,4 @@
 
 ## deprecated_event_handler
 
-> Using on:%name% to listen to the %name% event is is deprecated. Use the event attribute on%name% instead.
+> Using on:%name% to listen to the %name% event is is deprecated. Use the event attribute on%name% instead.
diff --git a/packages/svelte/messages/compile-warnings/options.md b/packages/svelte/messages/compile-warnings/options.md
@@ -1,3 +1,7 @@
+## options_deprecated_accessors
+
+The `accessors` option has been deprecated. It will have no effect in runes mode
+
 ## options_deprecated_immutable
 
 > The `immutable` option has been deprecated. It will have no effect in runes mode
diff --git a/packages/svelte/scripts/process-messages/index.js b/packages/svelte/scripts/process-messages/index.js
@@ -13,7 +13,9 @@ for (const category of fs.readdirSync('messages')) {
 	for (const file of fs.readdirSync(`messages/${category}`)) {
 		if (!file.endsWith('.md')) continue;
 
-		const markdown = fs.readFileSync(`messages/${category}/${file}`, 'utf-8');
+		const markdown = fs
+			.readFileSync(`messages/${category}/${file}`, 'utf-8')
+			.replace(/\r\n/g, '\n');
 
 		for (const match of markdown.matchAll(/## ([\w]+)\n\n([^]+?)(?=$|\n\n## )/g)) {
 			const [_, code, text] = match;
@@ -33,7 +35,9 @@ for (const category of fs.readdirSync('messages')) {
 }
 
 function transform(name, dest) {
-	const source = fs.readFileSync(new URL(`./templates/${name}.js`, import.meta.url), 'utf-8');
+	const source = fs
+		.readFileSync(new URL(`./templates/${name}.js`, import.meta.url), 'utf-8')
+		.replace(/\r\n/g, '\n');
 
 	const comments = [];
 
@@ -217,7 +221,8 @@ function transform(name, dest) {
 
 	fs.writeFileSync(
 		dest,
-		`/* This file is generated by scripts/process-messages.js. Do not edit! */\n\n` + module.code,
+		`/* This file is generated by scripts/process-messages/index.js. Do not edit! */\n\n` +
+			module.code,
 		'utf-8'
 	);
 }
diff --git a/packages/svelte/src/compiler/errors.js b/packages/svelte/src/compiler/errors.js
@@ -1,4 +1,4 @@
-/* This file is generated by scripts/process-messages.js. Do not edit! */
+/* This file is generated by scripts/process-messages/index.js. Do not edit! */
 
 /** @typedef {{ start?: number, end?: number }} NodeLike */
 // interface is duplicated between here (used internally) and ./interfaces.js
diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js
@@ -374,7 +374,7 @@ export function analyze_component(root, source, options) {
 		inject_styles: options.css === 'injected' || options.customElement,
 		accessors: options.customElement
 			? true
-			: !!options.accessors ||
+			: (runes ? false : !!options.accessors) ||
 				// because $set method needs accessors
 				!!options.legacy?.componentApi,
 		reactive_statements: new Map(),
@@ -395,8 +395,20 @@ export function analyze_component(root, source, options) {
 		source
 	};
 
-	if (!options.customElement && root.options?.customElement) {
-		w.options_missing_custom_element(root.options);
+	if (root.options) {
+		for (const attribute of root.options.attributes) {
+			if (attribute.name === 'accessors') {
+				w.options_deprecated_accessors(attribute);
+			}
+
+			if (attribute.name === 'customElement' && !options.customElement) {
+				w.options_missing_custom_element(attribute);
+			}
+
+			if (attribute.name === 'immutable') {
+				w.options_deprecated_immutable(attribute);
+			}
+		}
 	}
 
 	if (analysis.runes) {
diff --git a/packages/svelte/src/compiler/types/index.d.ts b/packages/svelte/src/compiler/types/index.d.ts
@@ -94,6 +94,7 @@ export interface CompileOptions extends ModuleCompileOptions {
 	 * If `true`, getters and setters will be created for the component's props. If `false`, they will only be created for readonly exported values (i.e. those declared with `const`, `class` and `function`). If compiling with `customElement: true` this option defaults to `true`.
 	 *
 	 * @default false
+	 * @deprecated This will have no effect in runes mode
 	 */
 	accessors?: boolean;
 	/**
@@ -107,6 +108,7 @@ export interface CompileOptions extends ModuleCompileOptions {
 	 * This allows it to be less conservative about checking whether values have changed.
 	 *
 	 * @default false
+	 * @deprecated This will have no effect in runes mode
 	 */
 	immutable?: boolean;
 	/**
diff --git a/packages/svelte/src/compiler/validate-options.js b/packages/svelte/src/compiler/validate-options.js
@@ -39,7 +39,7 @@ export const validate_component_options =
 		object({
 			...common,
 
-			accessors: boolean(false),
+			accessors: deprecate(w.options_deprecated_accessors, boolean(false)),
 
 			css: validator('external', (input) => {
 				if (input === true || input === false) {
diff --git a/packages/svelte/src/compiler/warnings.js b/packages/svelte/src/compiler/warnings.js
@@ -1,4 +1,4 @@
-/* This file is generated by scripts/process-messages.js. Do not edit! */
+/* This file is generated by scripts/process-messages/index.js. Do not edit! */
 
 import { getLocator } from 'locate-character';
 
@@ -544,6 +544,14 @@ export function invalid_self_closing_tag(node, name) {
 	w(node, "invalid_self_closing_tag", `Self-closing HTML tags for non-void elements are ambiguous — use <${name} ...></${name}> rather than <${name} ... />`);
 }
 
+/**
+ * e `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");
+}
+
 /**
  * The `immutable` option has been deprecated. It will have no effect in runes mode
  * @param {null | NodeLike} node
diff --git a/packages/svelte/src/internal/client/errors.js b/packages/svelte/src/internal/client/errors.js
@@ -1,4 +1,4 @@
-/* This file is generated by scripts/process-messages.js. Do not edit! */
+/* This file is generated by scripts/process-messages/index.js. Do not edit! */
 
 import { DEV } from 'esm-env';
 
diff --git a/packages/svelte/src/internal/client/render.js b/packages/svelte/src/internal/client/render.js
@@ -119,7 +119,7 @@ export function mount(component, options) {
  * 		events?: { [Property in keyof Events]: (e: Events[Property]) => any };
  *  	context?: Map<any, any>;
  * 		intro?: boolean;
- * 		recover?: false;
+ * 		recover?: boolean;
  * 	}} options
  * @returns {Exports}
  */
diff --git a/packages/svelte/src/internal/client/warnings.js b/packages/svelte/src/internal/client/warnings.js
@@ -1,4 +1,4 @@
-/* This file is generated by scripts/process-messages.js. Do not edit! */
+/* This file is generated by scripts/process-messages/index.js. Do not edit! */
 
 import { DEV } from 'esm-env';
 
diff --git a/packages/svelte/src/internal/shared/warnings.js b/packages/svelte/src/internal/shared/warnings.js
@@ -1,4 +1,4 @@
-/* This file is generated by scripts/process-messages.js. Do not edit! */
+/* This file is generated by scripts/process-messages/index.js. Do not edit! */
 
 import { DEV } from 'esm-env';
 
diff --git a/packages/svelte/tests/runtime-browser/test.ts b/packages/svelte/tests/runtime-browser/test.ts
@@ -94,7 +94,10 @@ async function run_test(
 
 						write(`${test_dir}/_output/client/${path.basename(args.path)}.js`, compiled.js.code);
 
-						compiled.warnings.forEach((warning) => warnings.push(warning));
+						compiled.warnings.forEach((warning) => {
+							if (warning.code === 'options_deprecated_accessors') return;
+							warnings.push(warning);
+						});
 
 						if (compiled.css !== null) {
 							compiled.js.code += `document.head.innerHTML += \`<style>${compiled.css.code}</style>\``;
@@ -179,6 +182,7 @@ async function run_test(
 			);
 		} else if (warnings.length) {
 			/* eslint-disable no-unsafe-finally */
+			console.warn(warnings);
 			throw new Error('Received unexpected warnings');
 		}
 	}
diff --git a/packages/svelte/tests/runtime-legacy/shared.ts b/packages/svelte/tests/runtime-legacy/shared.ts
@@ -2,7 +2,8 @@ import * as fs from 'node:fs';
 import { setImmediate } from 'node:timers/promises';
 import glob from 'tiny-glob/sync.js';
 import { createClassComponent } from 'svelte/legacy';
-import { flushSync } from 'svelte';
+import { proxy } from 'svelte/internal/client';
+import { flushSync, hydrate, mount, unmount } from 'svelte';
 import { render } from 'svelte/server';
 import { afterAll, assert, beforeAll } from 'vitest';
 import { compile_directory } from '../helpers.js';
@@ -43,6 +44,7 @@ export interface RuntimeTest<Props extends Record<string, any> = Record<string,
 		component: Props & {
 			[key: string]: any;
 		};
+		instance: Record<string, any>;
 		mod: any;
 		ok: typeof ok;
 		raf: {
@@ -120,7 +122,7 @@ export function runtime_suite(runes: boolean) {
 			return common_setup(cwd, runes, config);
 		},
 		async (config, cwd, variant, common) => {
-			await run_test_variant(cwd, config, variant, common);
+			await run_test_variant(cwd, config, variant, common, runes);
 		}
 	);
 }
@@ -148,7 +150,8 @@ async function run_test_variant(
 	cwd: string,
 	config: RuntimeTest,
 	variant: 'dom' | 'hydrate' | 'ssr',
-	compileOptions: CompileOptions
+	compileOptions: CompileOptions,
+	runes: boolean
 ) {
 	let unintended_error = false;
 
@@ -289,15 +292,30 @@ async function run_test_variant(
 				}
 			};
 
-			const instance = createClassComponent({
-				component: mod.default,
-				props: config.props,
-				target,
-				immutable: config.immutable,
-				intro: config.intro,
-				recover: config.recover ?? false,
-				hydrate: variant === 'hydrate'
-			});
+			let instance: any;
+			let props: any;
+
+			if (runes) {
+				props = proxy({ ...(config.props || {}) });
+
+				const render = variant === 'hydrate' ? hydrate : mount;
+				instance = render(mod.default, {
+					target,
+					props,
+					intro: config.intro,
+					recover: config.recover ?? false
+				});
+			} else {
+				instance = createClassComponent({
+					component: mod.default,
+					props: config.props,
+					target,
+					immutable: config.immutable,
+					intro: config.intro,
+					recover: config.recover ?? false,
+					hydrate: variant === 'hydrate'
+				});
+			}
 
 			// eslint-disable-next-line no-console
 			console.error = error;
@@ -327,7 +345,8 @@ async function run_test_variant(
 							htmlEqualWithOptions: assert_html_equal_with_options
 						},
 						variant,
-						component: instance,
+						component: runes ? props : instance,
+						instance,
 						mod,
 						target,
 						snapshot,
@@ -344,7 +363,11 @@ async function run_test_variant(
 					assert.fail('Expected a runtime error');
 				}
 			} finally {
-				instance.$destroy();
+				if (runes) {
+					unmount(instance);
+				} else {
+					instance.$destroy();
+				}
 
 				if (config.warnings) {
 					assert.deepEqual(warnings, config.warnings);
diff --git a/packages/svelte/tests/runtime-runes/samples/each-bind-this-member/_config.js b/packages/svelte/tests/runtime-runes/samples/each-bind-this-member/_config.js
@@ -1,6 +1,12 @@
 import { test } from '../../test';
 
 export default test({
+	get props() {
+		return {
+			items: [{ src: 'https://ds' }]
+		};
+	},
+
 	async test({ assert, target, component }) {
 		assert.equal(target.querySelector('img'), component.items[0].img);
 	}
diff --git a/packages/svelte/tests/runtime-runes/samples/each-bind-this-member/main.svelte b/packages/svelte/tests/runtime-runes/samples/each-bind-this-member/main.svelte
@@ -1,5 +1,5 @@
 <script>
-	let { items = $bindable([{ src: 'https://ds' }]) } = $props();
+	let { items } = $props();
 </script>
 
 {#each items as item, i}
diff --git a/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children-2/_config.js b/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children-2/_config.js
@@ -1,3 +1,4 @@
+import { flushSync } from 'svelte';
 import { test } from '../../test';
 
 export default test({
@@ -27,7 +28,7 @@ export default test({
 
 		logs.length = 0;
 
-		component.n += 1;
+		flushSync(() => (component.n += 1));
 
 		assert.deepEqual(logs, [
 			'parent: $effect.pre 1',
diff --git a/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children-3/_config.js b/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children-3/_config.js
@@ -1,3 +1,4 @@
+import { flushSync } from 'svelte';
 import { test } from '../../test';
 
 export default test({
@@ -22,7 +23,7 @@ export default test({
 
 		logs.length = 0;
 
-		component.n += 1;
+		flushSync(() => (component.n += 1));
 
 		assert.deepEqual(logs, [
 			'parent: render 1',
diff --git a/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children-4/_config.js b/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children-4/_config.js
@@ -1,3 +1,4 @@
+import { flushSync } from 'svelte';
 import { test } from '../../test';
 
 export default test({
@@ -27,7 +28,7 @@ export default test({
 
 		logs.length = 0;
 
-		component.n += 1;
+		flushSync(() => (component.n += 1));
 
 		assert.deepEqual(logs, [
 			'parent: $effect.pre 1',
diff --git a/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children-5/_config.js b/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children-5/_config.js
@@ -1,3 +1,4 @@
+import { flushSync } from 'svelte';
 import { test } from '../../test';
 
 export default test({
@@ -9,7 +10,7 @@ export default test({
 		assert.deepEqual(logs, ['$effect.pre 0', 'another $effect.pre 1', 'render n0', 'render i1']);
 
 		logs.length = 0;
-		component.n += 1;
+		flushSync(() => (component.n += 1));
 
 		assert.deepEqual(logs, ['$effect.pre 1', 'another $effect.pre 2', 'render n1', 'render i2']);
 	}
diff --git a/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children/_config.js b/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children/_config.js
@@ -1,3 +1,4 @@
+import { flushSync } from 'svelte';
 import { test } from '../../test';
 
 export default test({
@@ -23,7 +24,7 @@ export default test({
 
 		logs.length = 0;
 
-		component.n += 1;
+		flushSync(() => (component.n += 1));
 
 		assert.deepEqual(logs, [
 			'parent: $effect.pre 1',
diff --git a/packages/svelte/tests/runtime-runes/samples/props-default-value-lazy-accessors/_config.js b/packages/svelte/tests/runtime-runes/samples/props-default-value-lazy-accessors/_config.js
diff --git a/packages/svelte/tests/runtime-runes/samples/props-quoted/_config.js b/packages/svelte/tests/runtime-runes/samples/props-quoted/_config.js
diff --git a/packages/svelte/tests/runtime-runes/samples/props/_config.js b/packages/svelte/tests/runtime-runes/samples/props/_config.js
diff --git a/packages/svelte/tests/runtime-runes/samples/transition-evaluation/_config.js b/packages/svelte/tests/runtime-runes/samples/transition-evaluation/_config.js
diff --git a/packages/svelte/tests/validator/samples/tag-custom-element-options-missing/warnings.json b/packages/svelte/tests/validator/samples/tag-custom-element-options-missing/warnings.json
diff --git a/packages/svelte/types/index.d.ts b/packages/svelte/types/index.d.ts
diff --git a/sites/svelte-5-preview/src/routes/docs/content/03-appendix/02-breaking-changes.md b/sites/svelte-5-preview/src/routes/docs/content/03-appendix/02-breaking-changes.md

-Original file line number
+Diff line change
@@ @@ -0,0 +1,5 @@ @@
 +---
 +'svelte': patch
 +---
++
 +fix: mark `accessors` and `immutable` as deprecated
Original file line number	Diff line number	Diff line change
`@@ -16,4 +16,4 @@`
`16`	`16`
`17`	`17`	`## deprecated_event_handler`
`18`	`18`
`19`		`-> Using on:%name% to listen to the %name% event is is deprecated. Use the event attribute on%name% instead.`
	`19`	`+> Using on:%name% to listen to the %name% event is is deprecated. Use the event attribute on%name% instead.`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-/* This file is generated by scripts/process-messages.js. Do not edit! */`
	`1`	`+/* This file is generated by scripts/process-messages/index.js. Do not edit! */`
`2`	`2`
`3`	`3`	`/** @typedef {{ start?: number, end?: number }} NodeLike */`
`4`	`4`	`// interface is duplicated between here (used internally) and ./interfaces.js`