From 24777c335a85209e344fb2b61a60c7768165c747 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 5 Dec 2023 14:51:11 -0500 Subject: [PATCH] feat: disallow fallback values with bindings in runes mode (#9784) * disallow fallback values with bindings in runes mode * on second thoughts --------- Co-authored-by: Rich Harris --- .changeset/polite-ravens-study.md | 5 + .../phases/3-transform/client/utils.js | 10 +- packages/svelte/src/constants.js | 3 +- .../svelte/src/internal/client/runtime.js | 7 +- .../samples/props-bound-fallback/_config.js | 4 +- .../props-default-value-behavior/_config.js | 96 +++++-------------- .../props-default-value-behavior/inner.svelte | 5 +- .../props-default-value-behavior/main.svelte | 32 +------ .../_expected/client/index.svelte.js | 2 +- 9 files changed, 52 insertions(+), 112 deletions(-) create mode 100644 .changeset/polite-ravens-study.md diff --git a/.changeset/polite-ravens-study.md b/.changeset/polite-ravens-study.md new file mode 100644 index 0000000000..9f9c6f8717 --- /dev/null +++ b/.changeset/polite-ravens-study.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +feat: disallow fallback values with bindings in runes mode diff --git a/packages/svelte/src/compiler/phases/3-transform/client/utils.js b/packages/svelte/src/compiler/phases/3-transform/client/utils.js index 0227313214..35e200b06e 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/utils.js @@ -1,7 +1,11 @@ import * as b from '../../../utils/builders.js'; import { extract_paths, is_simple_expression } from '../../../utils/ast.js'; import { error } from '../../../errors.js'; -import { PROPS_CALL_DEFAULT_VALUE, PROPS_IS_IMMUTABLE } from '../../../../constants.js'; +import { + PROPS_CALL_DEFAULT_VALUE, + PROPS_IS_IMMUTABLE, + PROPS_IS_RUNES +} from '../../../../constants.js'; /** * @template {import('./types').ClientTransformState} State @@ -369,6 +373,10 @@ export function get_props_method(binding, state, name, default_value) { flags |= PROPS_IS_IMMUTABLE; } + if (state.analysis.runes) { + flags |= PROPS_IS_RUNES; + } + if (default_value) { // To avoid eagerly evaluating the right-hand-side, we wrap it in a thunk if necessary if (is_simple_expression(default_value)) { diff --git a/packages/svelte/src/constants.js b/packages/svelte/src/constants.js index d5daaf700b..f22a36f45f 100644 --- a/packages/svelte/src/constants.js +++ b/packages/svelte/src/constants.js @@ -6,7 +6,8 @@ export const EACH_IS_ANIMATED = 1 << 4; export const EACH_IS_IMMUTABLE = 1 << 6; export const PROPS_IS_IMMUTABLE = 1; -export const PROPS_CALL_DEFAULT_VALUE = 1 << 1; +export const PROPS_IS_RUNES = 1 << 1; +export const PROPS_CALL_DEFAULT_VALUE = 1 << 2; /** List of Element events that will be delegated */ export const DelegatedEvents = [ diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 29f1b733dd..462372f83f 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -2,7 +2,7 @@ import { DEV } from 'esm-env'; import { subscribe_to_store } from '../../store/utils.js'; import { EMPTY_FUNC, run_all } from '../common.js'; import { get_descriptor, get_descriptors, is_array } from './utils.js'; -import { PROPS_CALL_DEFAULT_VALUE, PROPS_IS_IMMUTABLE } from '../../constants.js'; +import { PROPS_CALL_DEFAULT_VALUE, PROPS_IS_IMMUTABLE, PROPS_IS_RUNES } from '../../constants.js'; export const SOURCE = 1; export const DERIVED = 1 << 1; @@ -1428,6 +1428,11 @@ export function prop_source(props_obj, key, flags, default_value) { let value = props[key]; const should_set_default_value = value === undefined && default_value !== undefined; + if (update_bound_prop && default_value !== undefined && (flags & PROPS_IS_RUNES) !== 0) { + // TODO consolidate all these random runtime errors + throw new Error('Cannot use fallback values with bind:'); + } + if (should_set_default_value) { value = // @ts-expect-error would need a cumbersome method overload to type this diff --git a/packages/svelte/tests/runtime-runes/samples/props-bound-fallback/_config.js b/packages/svelte/tests/runtime-runes/samples/props-bound-fallback/_config.js index 7d4900a745..6dc700cee5 100644 --- a/packages/svelte/tests/runtime-runes/samples/props-bound-fallback/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/props-bound-fallback/_config.js @@ -12,5 +12,7 @@ export default test({ await btn?.click(); assert.htmlEqual(target.innerHTML, `1`); - } + }, + + error: `Cannot use fallback values with bind:` }); diff --git a/packages/svelte/tests/runtime-runes/samples/props-default-value-behavior/_config.js b/packages/svelte/tests/runtime-runes/samples/props-default-value-behavior/_config.js index dc10606695..f4b1f09ec9 100644 --- a/packages/svelte/tests/runtime-runes/samples/props-default-value-behavior/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/props-default-value-behavior/_config.js @@ -10,7 +10,6 @@ export default test({ readonly: readonlyWithDefault: 1 binding: - bindingWithDefault: 1

@@ -20,27 +19,24 @@ export default test({ readonly: 0 readonlyWithDefault: 0 binding: 0 - bindingWithDefault: 0

- +

bindings undefined:

readonly: readonlyWithDefault: 1 binding: - bindingWithDefault: 1

- +

bindings defined:

readonly: 0 readonlyWithDefault: 0 binding: 0 - bindingWithDefault: 0

@@ -50,19 +46,13 @@ export default test({ readonly_undefined: readonlyWithDefault_undefined: binding_undefined: - bindingWithDefault_undefined: readonly_defined: 0 readonlyWithDefault_defined: 0 binding_defined: 0 - bindingWithDefault_defined: 0 bind_readonly_undefined: - bind_readonlyWithDefault_undefined: 1 bind_binding_undefined: - bind_bindingWithDefault_undefined: 1 bind_readonly_defined: 0 - bind_readonlyWithDefault_defined: 0 bind_binding_defined: 0 - bind_bindingWithDefault_defined: 0

@@ -93,64 +83,54 @@ export default test({ `

props undefined:

- readonly: + readonly: readonlyWithDefault: 1 binding: - bindingWithDefault:

- +

props defined:

readonly: 0 readonlyWithDefault: 0 binding: - bindingWithDefault:

- +

bindings undefined:

readonly: readonlyWithDefault: 1 binding: - bindingWithDefault:

- +

bindings defined:

readonly: 0 readonlyWithDefault: 0 binding: - bindingWithDefault:

- +

Main: readonly_undefined: readonlyWithDefault_undefined: binding_undefined: - bindingWithDefault_undefined: readonly_defined: 0 readonlyWithDefault_defined: 0 binding_defined: 0 - bindingWithDefault_defined: 0 bind_readonly_undefined: - bind_readonlyWithDefault_undefined: 1 bind_binding_undefined: - bind_bindingWithDefault_undefined: bind_readonly_defined: 0 - bind_readonlyWithDefault_defined: 0 bind_binding_defined: - bind_bindingWithDefault_defined:

- + ` @@ -169,61 +149,51 @@ export default test({ readonly: readonlyWithDefault: 1 binding: 5 - bindingWithDefault: 5

- +

props defined:

readonly: 0 readonlyWithDefault: 0 binding: 5 - bindingWithDefault: 5

- +

bindings undefined:

readonly: readonlyWithDefault: 1 binding: 5 - bindingWithDefault: 5

- +

bindings defined:

readonly: 0 readonlyWithDefault: 0 binding: 5 - bindingWithDefault: 5

- +

Main: readonly_undefined: readonlyWithDefault_undefined: binding_undefined: - bindingWithDefault_undefined: readonly_defined: 0 readonlyWithDefault_defined: 0 binding_defined: 0 - bindingWithDefault_defined: 0 bind_readonly_undefined: - bind_readonlyWithDefault_undefined: 1 bind_binding_undefined: 5 - bind_bindingWithDefault_undefined: 5 bind_readonly_defined: 0 - bind_readonlyWithDefault_defined: 0 bind_binding_defined: 5 - bind_bindingWithDefault_defined: 5

- + ` @@ -239,61 +209,51 @@ export default test({ readonly: 10 readonlyWithDefault: 10 binding: 10 - bindingWithDefault: 10

- +

props defined:

readonly: 10 readonlyWithDefault: 10 binding: 10 - bindingWithDefault: 10

- +

bindings undefined:

readonly: 10 readonlyWithDefault: 10 binding: 10 - bindingWithDefault: 10

- +

bindings defined:

readonly: 10 readonlyWithDefault: 10 binding: 10 - bindingWithDefault: 10

- +

Main: readonly_undefined: 10 readonlyWithDefault_undefined: 10 binding_undefined: 10 - bindingWithDefault_undefined: 10 readonly_defined: 10 readonlyWithDefault_defined: 10 binding_defined: 10 - bindingWithDefault_defined: 10 bind_readonly_undefined: 10 - bind_readonlyWithDefault_undefined: 10 bind_binding_undefined: 10 - bind_bindingWithDefault_undefined: 10 bind_readonly_defined: 10 - bind_readonlyWithDefault_defined: 10 bind_binding_defined: 10 - bind_bindingWithDefault_defined: 10

- + ` @@ -309,61 +269,51 @@ export default test({ readonly: readonlyWithDefault: binding: - bindingWithDefault:

- +

props defined:

readonly: readonlyWithDefault: binding: - bindingWithDefault:

- +

bindings undefined:

readonly: readonlyWithDefault: binding: - bindingWithDefault:

- +

bindings defined:

readonly: readonlyWithDefault: binding: - bindingWithDefault:

- +

Main: readonly_undefined: readonlyWithDefault_undefined: binding_undefined: - bindingWithDefault_undefined: readonly_defined: readonlyWithDefault_defined: binding_defined: - bindingWithDefault_defined: bind_readonly_undefined: - bind_readonlyWithDefault_undefined: bind_binding_undefined: - bind_bindingWithDefault_undefined: bind_readonly_defined: - bind_readonlyWithDefault_defined: bind_binding_defined: - bind_bindingWithDefault_defined:

- + ` diff --git a/packages/svelte/tests/runtime-runes/samples/props-default-value-behavior/inner.svelte b/packages/svelte/tests/runtime-runes/samples/props-default-value-behavior/inner.svelte index 80416bd346..9c66d82b7f 100644 --- a/packages/svelte/tests/runtime-runes/samples/props-default-value-behavior/inner.svelte +++ b/packages/svelte/tests/runtime-runes/samples/props-default-value-behavior/inner.svelte @@ -1,20 +1,17 @@

readonly: {readonly} readonlyWithDefault: {readonlyWithDefault} binding: {binding} - bindingWithDefault: {bindingWithDefault}

diff --git a/packages/svelte/tests/runtime-runes/samples/props-default-value-behavior/main.svelte b/packages/svelte/tests/runtime-runes/samples/props-default-value-behavior/main.svelte index d80295b002..a8793bb953 100644 --- a/packages/svelte/tests/runtime-runes/samples/props-default-value-behavior/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/props-default-value-behavior/main.svelte @@ -4,19 +4,13 @@ let readonly_undefined = $state(); let readonlyWithDefault_undefined = $state(); let binding_undefined = $state(); - let bindingWithDefault_undefined = $state(); let readonly_defined = $state(0); let readonlyWithDefault_defined = $state(0); let binding_defined = $state(0); - let bindingWithDefault_defined = $state(0); let bind_readonly_undefined = $state(); - let bind_readonlyWithDefault_undefined = $state(); let bind_binding_undefined = $state(); - let bind_bindingWithDefault_undefined = $state(); let bind_readonly_defined = $state(0); - let bind_readonlyWithDefault_defined = $state(0); let bind_binding_defined = $state(0); - let bind_bindingWithDefault_defined = $state(0);

props undefined:

@@ -24,7 +18,6 @@ readonly={readonly_undefined} readonlyWithDefault={readonlyWithDefault_undefined} binding={binding_undefined} - bindingWithDefault={bindingWithDefault_undefined} />

props defined:

@@ -32,23 +25,20 @@ readonly={readonly_defined} readonlyWithDefault={readonlyWithDefault_defined} binding={binding_defined} - bindingWithDefault={bindingWithDefault_defined} />

bindings undefined:

bindings defined:

@@ -56,55 +46,37 @@ readonly_undefined: {readonly_undefined} readonlyWithDefault_undefined: {readonlyWithDefault_undefined} binding_undefined: {binding_undefined} - bindingWithDefault_undefined: {bindingWithDefault_undefined} readonly_defined: {readonly_defined} readonlyWithDefault_defined: {readonlyWithDefault_defined} binding_defined: {binding_defined} - bindingWithDefault_defined: {bindingWithDefault_defined} bind_readonly_undefined: {bind_readonly_undefined} - bind_readonlyWithDefault_undefined: {bind_readonlyWithDefault_undefined} bind_binding_undefined: {bind_binding_undefined} - bind_bindingWithDefault_undefined: {bind_bindingWithDefault_undefined} bind_readonly_defined: {bind_readonly_defined} - bind_readonlyWithDefault_defined: {bind_readonlyWithDefault_defined} bind_binding_defined: {bind_binding_defined} - bind_bindingWithDefault_defined: {bind_bindingWithDefault_defined}

diff --git a/packages/svelte/tests/snapshot/samples/svelte-element/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/svelte-element/_expected/client/index.svelte.js index 55ccb739aa..bd4c1a30a1 100644 --- a/packages/svelte/tests/snapshot/samples/svelte-element/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/svelte-element/_expected/client/index.svelte.js @@ -6,7 +6,7 @@ import * as $ from "svelte/internal"; export default function Svelte_element($$anchor, $$props) { $.push($$props, true); - let tag = $.prop_source($$props, "tag", 1, 'hr'); + let tag = $.prop_source($$props, "tag", 3, 'hr'); /* Init */ var fragment = $.comment($$anchor); var node = $.child_frag(fragment);