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

* 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 a47827e57d.

* pass instance to tests

---------

Co-authored-by: Simon Holthausen <simon.holthausen@vercel.com>
pull/11318/head
Rich Harris 8 months ago committed by GitHub
parent 22b2c15280
commit 476f2172b6
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: mark `accessors` and `immutable` as deprecated

@ -16,4 +16,4 @@
## deprecated_event_handler ## 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.

@ -1,3 +1,7 @@
## options_deprecated_accessors
The `accessors` option has been deprecated. It will have no effect in runes mode
## options_deprecated_immutable ## options_deprecated_immutable
> The `immutable` 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

@ -13,7 +13,9 @@ for (const category of fs.readdirSync('messages')) {
for (const file of fs.readdirSync(`messages/${category}`)) { for (const file of fs.readdirSync(`messages/${category}`)) {
if (!file.endsWith('.md')) continue; 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)) { for (const match of markdown.matchAll(/## ([\w]+)\n\n([^]+?)(?=$|\n\n## )/g)) {
const [_, code, text] = match; const [_, code, text] = match;
@ -33,7 +35,9 @@ for (const category of fs.readdirSync('messages')) {
} }
function transform(name, dest) { 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 = []; const comments = [];
@ -217,7 +221,8 @@ function transform(name, dest) {
fs.writeFileSync( fs.writeFileSync(
dest, 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' 'utf-8'
); );
} }

@ -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 */ /** @typedef {{ start?: number, end?: number }} NodeLike */
// interface is duplicated between here (used internally) and ./interfaces.js // interface is duplicated between here (used internally) and ./interfaces.js

@ -374,7 +374,7 @@ export function analyze_component(root, source, options) {
inject_styles: options.css === 'injected' || options.customElement, inject_styles: options.css === 'injected' || options.customElement,
accessors: options.customElement accessors: options.customElement
? true ? true
: !!options.accessors || : (runes ? false : !!options.accessors) ||
// because $set method needs accessors // because $set method needs accessors
!!options.legacy?.componentApi, !!options.legacy?.componentApi,
reactive_statements: new Map(), reactive_statements: new Map(),
@ -395,8 +395,20 @@ export function analyze_component(root, source, options) {
source source
}; };
if (!options.customElement && root.options?.customElement) { if (root.options) {
w.options_missing_custom_element(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) { if (analysis.runes) {

@ -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`. * 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 * @default false
* @deprecated This will have no effect in runes mode
*/ */
accessors?: boolean; accessors?: boolean;
/** /**
@ -107,6 +108,7 @@ export interface CompileOptions extends ModuleCompileOptions {
* This allows it to be less conservative about checking whether values have changed. * This allows it to be less conservative about checking whether values have changed.
* *
* @default false * @default false
* @deprecated This will have no effect in runes mode
*/ */
immutable?: boolean; immutable?: boolean;
/** /**

@ -39,7 +39,7 @@ export const validate_component_options =
object({ object({
...common, ...common,
accessors: boolean(false), accessors: deprecate(w.options_deprecated_accessors, boolean(false)),
css: validator('external', (input) => { css: validator('external', (input) => {
if (input === true || input === false) { if (input === true || input === false) {

@ -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'; 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} ... />`); 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 * The `immutable` option has been deprecated. It will have no effect in runes mode
* @param {null | NodeLike} node * @param {null | NodeLike} node

@ -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'; import { DEV } from 'esm-env';

@ -119,7 +119,7 @@ export function mount(component, options) {
* events?: { [Property in keyof Events]: (e: Events[Property]) => any }; * events?: { [Property in keyof Events]: (e: Events[Property]) => any };
* context?: Map<any, any>; * context?: Map<any, any>;
* intro?: boolean; * intro?: boolean;
* recover?: false; * recover?: boolean;
* }} options * }} options
* @returns {Exports} * @returns {Exports}
*/ */

@ -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'; import { DEV } from 'esm-env';

@ -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'; import { DEV } from 'esm-env';

@ -94,7 +94,10 @@ async function run_test(
write(`${test_dir}/_output/client/${path.basename(args.path)}.js`, compiled.js.code); 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) { if (compiled.css !== null) {
compiled.js.code += `document.head.innerHTML += \`<style>${compiled.css.code}</style>\``; compiled.js.code += `document.head.innerHTML += \`<style>${compiled.css.code}</style>\``;
@ -179,6 +182,7 @@ async function run_test(
); );
} else if (warnings.length) { } else if (warnings.length) {
/* eslint-disable no-unsafe-finally */ /* eslint-disable no-unsafe-finally */
console.warn(warnings);
throw new Error('Received unexpected warnings'); throw new Error('Received unexpected warnings');
} }
} }

@ -2,7 +2,8 @@ import * as fs from 'node:fs';
import { setImmediate } from 'node:timers/promises'; import { setImmediate } from 'node:timers/promises';
import glob from 'tiny-glob/sync.js'; import glob from 'tiny-glob/sync.js';
import { createClassComponent } from 'svelte/legacy'; 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 { render } from 'svelte/server';
import { afterAll, assert, beforeAll } from 'vitest'; import { afterAll, assert, beforeAll } from 'vitest';
import { compile_directory } from '../helpers.js'; import { compile_directory } from '../helpers.js';
@ -43,6 +44,7 @@ export interface RuntimeTest<Props extends Record<string, any> = Record<string,
component: Props & { component: Props & {
[key: string]: any; [key: string]: any;
}; };
instance: Record<string, any>;
mod: any; mod: any;
ok: typeof ok; ok: typeof ok;
raf: { raf: {
@ -120,7 +122,7 @@ export function runtime_suite(runes: boolean) {
return common_setup(cwd, runes, config); return common_setup(cwd, runes, config);
}, },
async (config, cwd, variant, common) => { 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, cwd: string,
config: RuntimeTest, config: RuntimeTest,
variant: 'dom' | 'hydrate' | 'ssr', variant: 'dom' | 'hydrate' | 'ssr',
compileOptions: CompileOptions compileOptions: CompileOptions,
runes: boolean
) { ) {
let unintended_error = false; let unintended_error = false;
@ -289,15 +292,30 @@ async function run_test_variant(
} }
}; };
const instance = createClassComponent({ let instance: any;
component: mod.default, let props: any;
props: config.props,
target, if (runes) {
immutable: config.immutable, props = proxy({ ...(config.props || {}) });
intro: config.intro,
recover: config.recover ?? false, const render = variant === 'hydrate' ? hydrate : mount;
hydrate: variant === 'hydrate' 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 // eslint-disable-next-line no-console
console.error = error; console.error = error;
@ -327,7 +345,8 @@ async function run_test_variant(
htmlEqualWithOptions: assert_html_equal_with_options htmlEqualWithOptions: assert_html_equal_with_options
}, },
variant, variant,
component: instance, component: runes ? props : instance,
instance,
mod, mod,
target, target,
snapshot, snapshot,
@ -344,7 +363,11 @@ async function run_test_variant(
assert.fail('Expected a runtime error'); assert.fail('Expected a runtime error');
} }
} finally { } finally {
instance.$destroy(); if (runes) {
unmount(instance);
} else {
instance.$destroy();
}
if (config.warnings) { if (config.warnings) {
assert.deepEqual(warnings, config.warnings); assert.deepEqual(warnings, config.warnings);

@ -1,6 +1,12 @@
import { test } from '../../test'; import { test } from '../../test';
export default test({ export default test({
get props() {
return {
items: [{ src: 'https://ds' }]
};
},
async test({ assert, target, component }) { async test({ assert, target, component }) {
assert.equal(target.querySelector('img'), component.items[0].img); assert.equal(target.querySelector('img'), component.items[0].img);
} }

@ -1,5 +1,5 @@
<script> <script>
let { items = $bindable([{ src: 'https://ds' }]) } = $props(); let { items } = $props();
</script> </script>
{#each items as item, i} {#each items as item, i}

@ -1,3 +1,4 @@
import { flushSync } from 'svelte';
import { test } from '../../test'; import { test } from '../../test';
export default test({ export default test({
@ -27,7 +28,7 @@ export default test({
logs.length = 0; logs.length = 0;
component.n += 1; flushSync(() => (component.n += 1));
assert.deepEqual(logs, [ assert.deepEqual(logs, [
'parent: $effect.pre 1', 'parent: $effect.pre 1',

@ -1,3 +1,4 @@
import { flushSync } from 'svelte';
import { test } from '../../test'; import { test } from '../../test';
export default test({ export default test({
@ -22,7 +23,7 @@ export default test({
logs.length = 0; logs.length = 0;
component.n += 1; flushSync(() => (component.n += 1));
assert.deepEqual(logs, [ assert.deepEqual(logs, [
'parent: render 1', 'parent: render 1',

@ -1,3 +1,4 @@
import { flushSync } from 'svelte';
import { test } from '../../test'; import { test } from '../../test';
export default test({ export default test({
@ -27,7 +28,7 @@ export default test({
logs.length = 0; logs.length = 0;
component.n += 1; flushSync(() => (component.n += 1));
assert.deepEqual(logs, [ assert.deepEqual(logs, [
'parent: $effect.pre 1', 'parent: $effect.pre 1',

@ -1,3 +1,4 @@
import { flushSync } from 'svelte';
import { test } from '../../test'; import { test } from '../../test';
export default 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']); assert.deepEqual(logs, ['$effect.pre 0', 'another $effect.pre 1', 'render n0', 'render i1']);
logs.length = 0; logs.length = 0;
component.n += 1; flushSync(() => (component.n += 1));
assert.deepEqual(logs, ['$effect.pre 1', 'another $effect.pre 2', 'render n1', 'render i2']); assert.deepEqual(logs, ['$effect.pre 1', 'another $effect.pre 2', 'render n1', 'render i2']);
} }

@ -1,3 +1,4 @@
import { flushSync } from 'svelte';
import { test } from '../../test'; import { test } from '../../test';
export default test({ export default test({
@ -23,7 +24,7 @@ export default test({
logs.length = 0; logs.length = 0;
component.n += 1; flushSync(() => (component.n += 1));
assert.deepEqual(logs, [ assert.deepEqual(logs, [
'parent: $effect.pre 1', 'parent: $effect.pre 1',

@ -1,3 +1,4 @@
import { flushSync } from '../../../../src/index-client.js';
import { test } from '../../test'; import { test } from '../../test';
// Tests that default values only fire lazily when the prop is undefined, and every time // Tests that default values only fire lazily when the prop is undefined, and every time
@ -8,22 +9,32 @@ export default test({
p2: 0, p2: 0,
p3: 0 p3: 0
}, },
html: `<p>props: 0 0 0 0 1 1 1 1</p><p>log: nested.fallback_value,fallback_fn`, html: `<p>props: 0 0 0 0 1 1 1 1</p><p>log: nested.fallback_value,fallback_fn`,
async test({ assert, target, component }) { async test({ assert, target, component }) {
component.p0 = undefined; flushSync(() => {
component.p1 = undefined; component.p0 = undefined;
component.p2 = undefined; component.p1 = undefined;
component.p3 = undefined; component.p2 = undefined;
// Nuance: these are already undefined in the props, but we're setting them to undefined again, component.p3 = undefined;
// which calls the fallback value again, even if it will result in the same value. There's no way });
// to prevent this, and in practise it won't matter - and you shouldn't use accessors in runes mode anyway.
component.p4 = undefined; assert.htmlEqual(
component.p5 = undefined; target.innerHTML,
component.p6 = undefined; `<p>props: 1 1 1 1 1 1 1 1</p><p>log: nested.fallback_value,fallback_fn,nested.fallback_value,fallback_fn`
component.p7 = undefined; );
flushSync(() => {
component.p4 = undefined;
component.p5 = undefined;
component.p6 = undefined;
component.p7 = undefined;
});
assert.htmlEqual( assert.htmlEqual(
target.innerHTML, target.innerHTML,
`<p>props: 1 1 1 1 1 1 1 1</p><p>log: nested.fallback_value,fallback_fn,nested.fallback_value,fallback_fn,nested.fallback_value,fallback_fn` `<p>props: 1 1 1 1 1 1 1 1</p><p>log: nested.fallback_value,fallback_fn,nested.fallback_value,fallback_fn`
); );
} }
}); });

@ -1,3 +1,4 @@
import { flushSync } from 'svelte';
import { test } from '../../test'; import { test } from '../../test';
export default test({ export default test({
@ -8,7 +9,7 @@ export default test({
html: `hello`, html: `hello`,
async test({ assert, target, component }) { async test({ assert, target, component }) {
component['kebab-case'] = 'goodbye'; flushSync(() => (component['kebab-case'] = 'goodbye'));
assert.htmlEqual(target.innerHTML, `goodbye`); assert.htmlEqual(target.innerHTML, `goodbye`);
} }
}); });

@ -1,3 +1,4 @@
import { flushSync } from 'svelte';
import { test } from '../../test'; import { test } from '../../test';
export default test({ export default test({
@ -9,14 +10,14 @@ export default test({
default2: undefined default2: undefined
}; };
}, },
html: `x 1 2 3 z`, html: `x 1 2 3 z`,
async test({ assert, target, component }) { async test({ assert, target, component }) {
component.foo = 'y'; flushSync(() => (component.foo = 'y'));
assert.htmlEqual(target.innerHTML, `y 1 2 3 z`); assert.htmlEqual(target.innerHTML, `y 1 2 3 z`);
// rest props don't generate accessors, so we need to use $set flushSync(() => (component.bar = 'w'));
await component.$set({ bar: 'w' });
assert.htmlEqual(target.innerHTML, `y 1 2 3 w`); assert.htmlEqual(target.innerHTML, `y 1 2 3 w`);
} }
}); });

@ -1,3 +1,4 @@
import { flushSync } from '../../../../src/index-client.js';
import { test } from '../../test'; import { test } from '../../test';
export default test({ export default test({
@ -5,8 +6,10 @@ export default test({
const div = /** @type {HTMLDivElement & { foo?: number }} */ (target.querySelector('div')); const div = /** @type {HTMLDivElement & { foo?: number }} */ (target.querySelector('div'));
assert.equal(div.foo, undefined); assert.equal(div.foo, undefined);
component.foo = 2; flushSync(() => {
component.visible = false; component.foo = 2;
component.visible = false;
});
assert.equal(div.foo, 2); assert.equal(div.foo, 2);
} }
}); });

@ -4,11 +4,11 @@
"message": "The `customElement` option is used when generating a custom element. Did you forget the `customElement: true` compile option?", "message": "The `customElement` option is used when generating a custom element. Did you forget the `customElement: true` compile option?",
"start": { "start": {
"line": 1, "line": 1,
"column": 0 "column": 16
}, },
"end": { "end": {
"line": 1, "line": 1,
"column": 49 "column": 46
} }
} }
] ]

@ -345,7 +345,7 @@ declare module 'svelte' {
events?: { [Property in keyof Events]: (e: Events[Property]) => any; } | undefined; events?: { [Property in keyof Events]: (e: Events[Property]) => any; } | undefined;
context?: Map<any, any> | undefined; context?: Map<any, any> | undefined;
intro?: boolean | undefined; intro?: boolean | undefined;
recover?: false | undefined; recover?: boolean | undefined;
}): Exports; }): Exports;
/** /**
* Unmounts a component that was previously mounted using `mount` or `hydrate`. * Unmounts a component that was previously mounted using `mount` or `hydrate`.
@ -623,6 +623,7 @@ declare module 'svelte/compiler' {
* 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`. * 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 * @default false
* @deprecated This will have no effect in runes mode
*/ */
accessors?: boolean; accessors?: boolean;
/** /**
@ -636,6 +637,7 @@ declare module 'svelte/compiler' {
* This allows it to be less conservative about checking whether values have changed. * This allows it to be less conservative about checking whether values have changed.
* *
* @default false * @default false
* @deprecated This will have no effect in runes mode
*/ */
immutable?: boolean; immutable?: boolean;
/** /**
@ -2419,6 +2421,7 @@ declare module 'svelte/types/compiler/interfaces' {
* 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`. * 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 * @default false
* @deprecated This will have no effect in runes mode
*/ */
accessors?: boolean; accessors?: boolean;
/** /**
@ -2432,6 +2435,7 @@ declare module 'svelte/types/compiler/interfaces' {
* This allows it to be less conservative about checking whether values have changed. * This allows it to be less conservative about checking whether values have changed.
* *
* @default false * @default false
* @deprecated This will have no effect in runes mode
*/ */
immutable?: boolean; immutable?: boolean;
/** /**

@ -129,6 +129,10 @@ Exports from runes mode components cannot be bound to directly. For example, hav
In Svelte 4 syntax, every property (declared via `export let`) is bindable, meaning you can `bind:` to it. In runes mode, properties are not bindable by default: you need to denote bindable props with the [`$bindable`](/docs/runes#$bindable) rune. In Svelte 4 syntax, every property (declared via `export let`) is bindable, meaning you can `bind:` to it. In runes mode, properties are not bindable by default: you need to denote bindable props with the [`$bindable`](/docs/runes#$bindable) rune.
### `accessors` option is ignored
Setting the `accessors` option to `true` makes properties of a component directly accessible on the component instance. In runes mode, properties are never accessible on the component instance. You can use component exports instead if you need to expose them.
## Other breaking changes ## Other breaking changes
### Stricter `@const` assignment validation ### Stricter `@const` assignment validation

Loading…
Cancel
Save