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

Add a (singular) $prop rune #9241

Closed
aradalvand opened this issue Sep 21, 2023 · 97 comments
Closed

Add a (singular) $prop rune #9241

aradalvand opened this issue Sep 21, 2023 · 97 comments
Labels

Comments

@aradalvand
Copy link

aradalvand commented Sep 21, 2023

Describe the problem

This came up multiple times in the Discord server, so I figured I'd create an issue for it:
In v5's runes mode, the only way to declare props is by using the $props rune, like so:

let { someProp = 'default' } = $props();

This looks good unless you're using TypeScript, in which case in order to type your props, you'll have to pass a type argument to $props, what this means in effect is that you'll be duplicating your prop names:

let { someProp = 'default' } = $props<{ someProp: string }>();

Needless to say this is not very pretty, feels a bit overly verbose, it gets even more problematic when you have many props:

let {
    foo = 'default',
    bar,
    buzz = 123,
} = $props<{
    foo: string;
    bar: number;
    buzz: number;
}>();

Here, you have to traverse quite a bit of distance with your eyes in order to find a specific prop's type (e.g. bar), because the destructured variable and the corresponding type could potentially be far from each other, and you'll have to "switch" your eyes to an entirely different "list", if you will.

In addition, this format yields a weird order of name => default-value => name => type, and when you compare all of this to the old way of declaring props with types, the difference becomes apparent:

export let foo: string = 'default';
export let bar: number;
export let buzz: number = 123;

Which is more concise, of course, but the order is more logical as well: name => type => default-value.

Describe the proposed solution

Introduce a singular $prop rune, like so:

let someProp = $prop<string>('default');

Which, once again, is less verbose and has a more logical order of symbols; the type of each prop is always next to it on the same line, and you don't have to repeat the prop name.

Alternatives considered

  • Just using the $props rune, which suffers from the aforementioned problems.

Importance

nice to have

@aradalvand aradalvand changed the title Support the (singular) $prop rune Add a (singular) $prop rune Sep 21, 2023
@theodorejb
Copy link
Contributor

Would it be possible for this to work with a type declaration directly on the variable rather than a generic? E.g.:

let someProp: string = $prop('default');

@Snailedlt
Copy link
Contributor

If "hijacking" TypeScript's syntax is an option, I also think this should be a considered syntax:

let {
    foo: string = 'default',
    bar: number,
    buzz: number = 123,
} = $props();

No more duplicate prop names, and the order of name-> type -> value is preserved.

@brunnerh
Copy link
Member

brunnerh commented Sep 21, 2023

Would it be possible for this to work with a type declaration directly on the variable rather than a generic? E.g.:

let someProp: string = $prop('default');

Generally not, that sort of annotation will only be checked for compatibility with the value being assigned, not change the type.

(Example)


@Snailedlt That should further simplify to:

let {
    foo = 'default',
    bar: number,
    buzz = 123,
} = $props();

@aradalvand
Copy link
Author

aradalvand commented Sep 21, 2023

@Snailedlt That syntax is already reserved by JavaScript for specifying aliases for the destructured properties. It can't be used for specifying types.

@aradalvand
Copy link
Author

aradalvand commented Sep 21, 2023

Actually, one problem with the singular $prop approach I proposed is that you won't be able to use reserved keywords as prop names — previously you could do that like so:

let classProp: string;
export { classProp as class };

And with the plural $props rune you could do it like this:

let { class: classProp } = $props();

But it doesn't seem like you'd be able to do the same thing with the singular $prop, as I proposed it; this, coupled with the fact the ...rest syntax won't work either (unless we were to also add a $restProps rune or something like that), makes me a little hesitant.

@brunnerh
Copy link
Member

brunnerh commented Sep 21, 2023

Edit: The following is now an error.


You can use $props multiple times, by the way.

let { foo = 'default' } = $props<{ foo: string }>();
let { bar } = $props<{ bar: number }>();
let { buzz = 123 } = $props<{ buzz: number }>();

Bit verbose, though.

@aradalvand
Copy link
Author

@brunnerh But the prop names are still being repeated, which is the main problem.

@Snailedlt
Copy link
Contributor

@Snailedlt That syntax is already reserved by JavaScript for specifying aliases for the destructured properties. It can't be used for specifying types.

@aradalvand Oh, didn't know that, how about this then?

let {
    foo<string> = 'default',
    bar<number>,
    buzz<number> = 123,
} = $props();

@aradalvand
Copy link
Author

aradalvand commented Sep 21, 2023

@Snailedlt Again, invalid syntax.

@TheHadiAhmadi
Copy link

TheHadiAhmadi commented Sep 21, 2023

What do you think about this syntax?

const klass = $prop<string>("class");
const htmlFor = $prop<string>("for");
const myNumber = $prop<number>("my-number"); // also kebab-case prop names would be possible
<MyComponent class="something" for="input-1" my-number={123} />

@aradalvand
Copy link
Author

aradalvand commented Sep 21, 2023

@TheHadiAhmadi Could work. We could also just use ?? [default] for specifying the default value.

let klass = $prop<string>('class') ?? 'default';

Update: I think ?? falls back to the right operand if the left operand is undefined or null, which is different from how default values in object destructuring work (e.g. let { foo = 'default' } = $props()), where it only falls back to the default value if the property is undefined (but not null). So, this idea may not be feasible either, given that.

@benmccann benmccann added this to the 5.x milestone Sep 21, 2023
@benmccann benmccann removed this from the 5.x milestone Sep 21, 2023
@Prinzhorn
Copy link
Contributor

I'm not even using TypeScript and would prefer having all props neatly separated. In the same way that I would never do let foo, bar; or export let foo, bar; because it is way less scannable (the eyes need to do more horizontal movement instead of just going down and scan over all declarations, yes I'm aware you can split the destructured object over multiple lines). Given that Svelte always follows the "code is read more often than it is written" mantra it is odd that the noise of destructuring was chosen over single props. But I understand that this is my personal preference and obviously we need to keep the $props rune for all kinds of reasons and edge cases.

I'll throw the following in the mix, why not (runes seem to be bending what's considered JavaScript anyway and declaring prop names as strings does not sit well with me):

const regularRequired = $prop<string>();
const reservedClassKeyword = $prop.class<string>();
const withDefault = $prop<number>(10);

But I kind of hate it, you can't immediately see the props this component receives, you need to pay attention to the .class. But I guess that's true for most suggestions, destructuring makes this the easiest due to proximity.

What if $prop also allows destructuring, but only to objects with a single property? These are compiler instructions after all, but we might anger the JavaScript police more and more as I speak:

const regularRequired = $prop<string>();
const { class: reservedClassKeyword } = $prop<string>();
const withDefault = $prop<number>(10);

@dummdidumm
Copy link
Member

We did not add it yet for a couple of reasons:

  • no good way to alias reserved names like class (there's a couple of proposals in this thread, but as the authors themselves point out none of them are great)
  • if only $prop() or $props() can be used (i.e. you can't use both in the same component), then you're in for annoying refactors from $prop() to $props() as soon as you need rest props, or aliases, or want to specify that the component implements a certain interface
  • if both can be used, there need to be elaborate rules in place to deconflict types and definitions (for example so that you can't do const x = $prop<string>(); const { x: _, ...rest } = $props<{ x: number; .. }>())

@look997
Copy link

look997 commented Sep 23, 2023

This can be hidden in $props.single(). There is $effect.pre(), so it is similar....

@WaltzingPenguin
Copy link

Is it possible the priorities and trade offs decided are just wrong? Writing a TypeScript component is really common, exporting class is really uncommon (and I'd even argue an anti-pattern in 90% of cases).

Making the stuff I read and write every single time I open a file in a TypeScript project more difficult in exchange for making something I seldom do a little less kludgy (you still have to alias it after all), is a horrible trade off.

@brunnerh
Copy link
Member

brunnerh commented Sep 25, 2023

exporting class is really uncommon

It's really not, I do that all the time when wrapping base elements.
Component libraries require this a lot unless they access $$props.

@aradalvand
Copy link
Author

aradalvand commented Sep 25, 2023

exporting class is really uncommon (and I'd even argue an anti-pattern in 90% of cases).

I strenuously disagree with both of those assertions — I would refer you to #6972.

@MaximSagan
Copy link

MaximSagan commented Sep 28, 2023

I'm not even using TypeScript and would prefer having all props neatly separated. In the same way that I would never do let foo, bar; or export let foo, bar; because it is way less scannable (the eyes need to do more horizontal movement instead of just going down and scan over all declarations, yes I'm aware you can split the destructured object over multiple lines). Given that Svelte always follows the "code is read more often than it is written" mantra it is odd that the noise of destructuring was chosen over single props. But I understand that this is my personal preference and obviously we need to keep the $props rune for all kinds of reasons and edge cases.

I'll throw the following in the mix, why not (runes seem to be bending what's considered JavaScript anyway and declaring prop names as strings does not sit well with me):

const regularRequired = $prop<string>();
const reservedClassKeyword = $prop.class<string>();
const withDefault = $prop<number>(10);

But I kind of hate it, you can't immediately see the props this component receives, you need to pay attention to the .class.

I feel like we're getting closer, but do we need to restrict ourself to only one "rune" for this task? As popular as reserved words may be in some cases (I agree "class" certainly is - and I don't think it should be considered an antipattern), for the most part they're still an edge case, so maybe better not to pollute the behavior of the default non-aliased $prop and just provide an additional "rune", e.g. something like $aliasedProp?

const regularRequired = $prop<string>();
const regularWithDefault = $prop<number>(10);
const aliasedRequired = $aliasedProp('class')<string>();
const aliasedWithDefault = $aliasedProp('catch')<boolean>(false);

An alternative signature for $aliasedProp could take two parameters, <T>(alias: string, defaultValue?: T) => T, but I slightly prefer the curried version I used in the example, (alias: string) => <T>(defaultValue?: T) => T, because with this version it's less likely the alias would be confused for the default value.

@intrnl
Copy link

intrnl commented Sep 28, 2023

Agree with having $aliasedProp or $prop.alias but highly disagree on the curry syntax as it looks busier with them.

@Jak-Ch-ll
Copy link

I feel like we're getting closer, but do we need to restrict ourself to only one "rune" for this task? As popular as reserved words may be in some cases (I agree "class" certainly is - and I don't think it should be considered an antipattern), for the most part they're still an edge case, so maybe better not to pollute the behavior of the default non-aliased $prop and just provide an additional "rune", e.g. something like $aliasedProp?

const regularRequired = $prop<string>();
const regularWithDefault = $prop<number>(10);
const aliasedRequired = $aliasedProp('class')<string>();
const aliasedWithDefault = $aliasedProp('catch')<boolean>(false);

An alternative signature for $aliasedProp could take two parameters, <T>(alias: string, defaultValue?: T) => T, but I slightly prefer the curried version I used in the example, (alias: string) => <T>(defaultValue?: T) => T, because with this version it's less likely the alias would be confused for the default value.

How about the following:

const regularRequired = $prop<string>();
const regularWithDefault = $prop(10);

const aliasedRequired = $prop<string>().as('class');
const aliasedWithDefault = $prop(false).as('catch');

Since the exposed property names for the component need to be statically analyzed anyway, we can do pretty much anything, as long as it's valid js syntax.

@Jak-Ch-ll
Copy link

When it comes to this discussion it's also probably worthwhile to check out the unofficial experimental defineProp macro for Vue: https://vue-macros.sxzz.moe/macros/define-prop.html

@brunnerh
Copy link
Member

brunnerh commented Oct 9, 2023

we can do pretty much anything, as long as it's valid js syntax.

That is not quite correct. In terms of types, runes expose the underlying values, so you cannot have chaining like that and still preserve the simple case without it.

E.g. $prop(10) will have the type number and thus no as function, so unless all types are polluted with those fake functions, that will cause issues. (Even then I was unable to get the typing to work as the this return type appears to not resolve correctly.)

@Jak-Ch-ll
Copy link

we can do pretty much anything, as long as it's valid js syntax.

That is not quite correct. In terms of types, runes expose the underlying values, so you cannot have chaining like that and still preserve the simple case without it.

E.g. $prop(10) will have the type number and thus no as function, so unless all types are polluted with those fake functions, that will cause issues. (Even then I was unable to get the typing to work as the this return type appears to not resolve correctly.)

Ah, you are right, I did not take the types into consideration. Then let me throw in another idea, which is a somewhat revised version of yours and takes additional influences from the Vue defineProp macro:

const aliasedRequired = $prop<string>(undefined, {
  name: 'class'
})
const aliasedWithDefault = $prop(false, {
  name: 'catch'
})

The main advantages are, that there is no need for an additional rune and, at least from my point of view, it's highly readable and intuitive what's going on.

The obvious disadvantage is that the syntax is somewhat more verbose, especially having to add undefined for required props. But as these renames should be the exception, I don't see this as a massive downside.

Another advantage is, that this syntax is extendable for future features, e.g. a runtime validation, similar to Vue

@Not-Jayden
Copy link
Contributor

Ah, you are right, I did not take the types into consideration. Then let me throw in another idea, which is a somewhat revised version of yours and takes additional influences from the Vue defineProp macro:

const aliasedRequired = $prop<string>(undefined, {
  name: 'class'
})
const aliasedWithDefault = $prop(false, {
  name: 'catch'
})

One thing I'm not certain about with this syntax is, without Typescript, how is Svelte supposed to know that aliasRequired is required, or if it has just been defaulted to the value of undefined?

@brunnerh
Copy link
Member

Good point regarding the default/required issue.
We could maybe use a different syntax here, utilizing ?? 🤔

declare function $prop<T = undefined>(options?: any): T;

let required = $prop<number>();
let withDefault = $prop<number>() ?? 42;
let withDefaultImplicit = $prop() ?? 42;

let aliasedRequired = $prop<string>({ name: 'class '});
let aliasedWithDefault = $prop({ name: 'class '}) ?? 'flex';

// ?? removes undefined and null from the type
let requiredAllowUndefined = $prop<number | undefined>();
let requiredAllowNull = $prop<number | null>();
let withDefaultAllowUndefined = $prop() ?? (42 as number | undefined);
let withDefaultAllowNull = $prop() ?? (42 as number | null);

TS Playground

@7nik
Copy link

7nik commented Oct 12, 2023

  1. ?? will replace both null and undefined with the default value, while the default value at restructuring replaces only undefined.
  2. The as operator does type casting with either type narrowing or extending and shouldn't be used at variable initialization unless you really want it. Example.

@adiguba
Copy link
Contributor

adiguba commented May 15, 2024

@Not-Jayden : I like it.

But two remarks :

  • there should be a way to differentiate between optional and required props (a $req rune ?)
  • and a way to type the spreaded props (by typing the result like now ?)

Perhaps something like that

let {
  foo = $prop<string>(),              // foo is a optionnal string (undefined by default)
  bar = $prop<number>(42),            // bar is an optionnal number (42 by default)
  baz = $prop<string>('The Answer'),  // baz is an optionnal string ('The Answer' by default) 
  id = $req<string>(),                // baw is a required string
  anotherProp,                        // anotherProp is typed based on TypeOfOthersProps 
  ...props                            // others props, typed using TypeOfOthersProps 
} : TypeOfOthersProps = $props();

would be equivalent to the following code :

let {
  foo,
  bar = 42, 
  baz = 'The Answer',
  id,
  anotherProp,
  ...props 
} : {
  foo?: string,
  bar?: number, 
  baz?: string,
  id: string
} & TypeOfOthersProps  = $props();

This could work...

@Not-Jayden
Copy link
Contributor

@adiguba Good considerations, didn't really think through the edges of it too hard.

I think we both had different assumptions of how the type would be resolved, which might hint that the behaviour is a bit too magical. My expectation would be that foo is a required property by virtue of not having a default value passed as an argument.

Also I think it starts to break down if you try mix the type annotation after the destructured object with the $prop rune as the annotation will override the inferred type returned from $props(). I'd maybe introduce $prop.optional() and $prop.rest() for those cases.

let {
  foo = $prop<string>(),              // required string
  bar = $prop<number>(42),            // optional number
  baz = $prop<string>('The Answer'),  // optional string
  id = $prop.optional<number>(),      // optional number (i.e. type of id is number | undefined, but you can't pass undefined as a prop value)
  props = $prop.rest<OtherProps>(),   // rest of the props
} = $props();

Alternatively (additionally?) maybe there could be something like $props.Type<T extends Record<string, unknown>> which resolves to the type of the props defined with $prop(), as well as allowing you to pass in the additional props that would be assigned to rest.

let {
  foo = $prop<string>(),              // required string
  bar = $prop<number>(42),            // optional number
  baz = $prop<string>('The Answer'),  // optional string
  id,                                 // optional number (via $prop.Type)
  ...props,                           // {qux: string} (via $prop.Type)
}: $props.Type<{id?: number, qux: string}> = $props();

// $prop.Type<{id?: number, qux: string}> is equivalent to
// {foo: string, bar?: number, baz?: string} & {id?: number, qux: string}

Feeling a little less convinced of this approach with the additional complexity involved though.

@ivodolenc
Copy link

I recently started using the new Svelte 5 version and bumped into this props thing. It works as expected, but I'd say it gets a little annoying when using TypeScript.

Some of the props like data and snippets/slots (children) are automatically inferring types and thats really awesome.

But lets see this simple example bellow:

<!-- component.svelte -->

<script lang="ts">
  let { data, children } = $props()
</script>

<div>
  <h1>{data.title}<h1>
  {@render children()}
</div>

It works as expected and the types are correct, no additional work is required.


How to deal it when you only need to type one or two props and leave the rest to automatic inferring so we are not repeating or importing additional types every time?

<!-- component.svelte -->

<script lang="ts">
  import type { Snippet } from 'svelte'
  import type { ContentPages, ContentMenu } from '$types'

  type Props = {
    data: {
      title: string
      pages: ContentPages[]
      menu: ContentMenu
      // ...
    }
    children: Snippet
    x: number
    y: string
  }

  let { data, children, x, y }: Props = $props()
</script>

As you can see above, I now have to explicitly type all the Props in order to be able to add two new props x and y.

It's not a big deal if you have a few components, but when you have a lot of components and more complex nested data and other props, it can become very messy and time consuming.


As the already suggested above, one should definitely consider all the options that exist to be able to integrate the individual use of props.

<!-- component.svelte -->

<script lang="ts">
  let data = $prop() // automatically infer types
  let children = $prop() // automatically infer types
  let x: number = $prop() // explicitly sets the type
  let y: string = $prop() // explicitly sets the type
</script>

or

<!-- component.svelte -->

<script lang="ts">
  let {
    data = $prop(), // automatically infer types
    children = $prop(), // automatically infer types
    x = $prop<number>(), // explicitly sets the type
    y = $prop<string>(), // explicitly sets the type
  } = $props()
</script>

Mostly the syntax is less important in this case, it is important that props can be added individually if needed.

@bfanger
Copy link
Contributor

bfanger commented Jun 30, 2024

For aliasing I like to propose the following syntax:

<script lang="ts">
  let className: string | undefined = $prop["class"](undefined); 

But only allow that for the limited set of javascript reserved keywords.
That allows for rune updates later because $prop["bindable"] / $prop.bindable would not be considered an alias.

@ananduremanan
Copy link

ananduremanan commented Jul 17, 2024

Maybe off topic and some freak bot definitely hide this comment. But, I gotta say this "Thanks for making svelte unusable with this runes and crazy things, svelte used to be great with svelte 4".

struggling to rewrite my entire component library with $props rune and typescript.

@Malix-Labs
Copy link

Malix-Labs commented Jul 17, 2024

You've probably not reached the point where runes makes things easier

@ottomated
Copy link
Contributor

I've put together a preprocessor that implements a version of this rune: https://github.com/ottomated/svelte5-prop-rune

Some examples:

const simple: string = $prop();
const optional: string = $prop('fallback value');

const value: number = $prop.bindable();
const class_: string = $prop().as('class');

const rest: HTMLButtonAttributes = $prop.rest();

I think this addresses your concerns, @dummdidumm -

no good way to alias reserved names like class (there's a couple of proposals in this thread, but as the authors themselves point out none of them are great)

I think this proposal is clean and type-safe. Open to feedback.

if only $prop() or $props() can be used (i.e. you can't use both in the same component), then you're in for annoying refactors from $prop() to $props() as soon as you need rest props, or aliases, or want to specify that the component implements a certain interface

This allows for rest props and aliases without refactoring. Having a component implement an interface seems rare, but I think unavoidable if $prop and $props are exclusive.

if both can be used, there need to be elaborate rules in place to deconflict types and definitions (for example so that you can't do const x = $prop(); const { x: _, ...rest } = $props<{ x: number; .. }>())

Agree that this would be pretty complicated, but I don't think that's an excuse for not doing it :) I would be happy to help on any development efforts around this.

@david-plugge
Copy link

This is soooo much more boilerplate... I dont understand how this is would benefit

@Not-Jayden
Copy link
Contributor

To add more weight the option of doing nothing, it's worth noting maybe this problem will be solved in TypeScript-land one day if this discussion ever progresses microsoft/TypeScript#29526

i.e. one day it might just be able to be typed like this

let {
    foo<string>,
    bar<number?> = 42,
    class<string?>: className,
} = $props();

but I'm certainly not holding my breath.

@FeldrinH
Copy link

To add more weight the option of doing nothing, it's worth noting maybe this problem will be solved in TypeScript-land one day if this discussion ever progresses microsoft/TypeScript#29526

It is theoretically possible that TypeScript may eventually solve this problem on their end, but I wouldn't count that as a particularly meaningful argument in favour of doing nothing in Svelte. That particular TypeScript discussion has been open since 2019 and seems to be stuck in limbo for the foreseeable future. There seems to be no clear consensus on the syntax and as far as I can tell the TypeScript development team has never acknowledged or commented on any of the proposals in that discussion.

@ottomated
Copy link
Contributor

@Rich-Harris I'd like to give implementing this a shot, do you have any comments on this spec?

@Rich-Harris
Copy link
Member

Please don't — we have no intention of changing how props work. The lack of a singular $prop rune isn't an oversight, it's the result of many, many hours of careful deliberation. I understand that it feels less streamlined in simple cases, but there is a significant cost to having multiple approaches.

With the current design, you can use your existing JavaScript/TypeScript knowledge. Need to rename a prop? You can do name: local. Need a fallback value? You can do value = 'fallback'. Need rest props? You can do ...rest. Need to make a value required even though it's part of those rest props? Well, just don't use ?: in the type annotation. Need to do advanced stuff with unions and whatever? You can. And so on. The only special thing you really need to learn is $bindable(), because that concept doesn't exist in JS/TS.

That all stands in contrast to the various proposals here, which involve new runes and new rules. It gets particularly complicated if you allow $prop() and $props() in the same component (and if you don't, refactoring becomes very tedious).

It's worth noting that while the example above...

const simple: string = $prop();
const optional: string = $prop('fallback value');

const value: number = $prop.bindable();
const class_: string = $prop().as('class');

const rest: HTMLButtonAttributes = $prop.rest();

...is fewer lines than today's version with Prettier applied...

const {
  simple,
  optional = 'fallback value',
  value = $bindable(),
  class: class_,
  ...rest
}: {
  simple: string;
  optional?: string;
  value: number;
  class: string;
} & HTMLButtonAttributes = $props();

...it's actually more characters. When I look at the first one, it takes more effort to understand which non-rest props exist — I need to look at the variable declaration and also check to see if there's an .as('...') call. Personally I also find it much easier to understand which props have what types when I look at the second example than the first, where the types are jumbled up with the rest of the code.

And of course I can do this sort of thing, and reference props.simple and props.class if I so choose, without also having to carefully update the type of const rest:

const {
- simple,
  optional = 'fallback value',
  value = $bindable(),
- class: class_,
  ...props
}: {
  simple: string;
  optional?: string;
  value: number;
  class: string;
} & HTMLButtonAttributes = $props();

In fact (apologies for not updating the thread with this change), a while back we made it possible to forego the destructuring altogether in the common case that you don't need optional or bindable values, so in many cases you can do this sort of thing:

let props: { a: string; b: number } = $props();

// exactly equivalent to this
let { ...props }: { a: string; b: number } = $props();

To reiterate a point that has been made elsewhere: this is just how TypeScript works. Every single time you declare a function with destructured arguments anywhere in your codebase, you have to do the exact same thing (but worse, if you use explicit return types):

function foo({
  a = 1,
  b = 2,
  c = 3
}: {
  a: number;
  b: number;
  c: number;
}): {
  d: number;
  e: number;
  f: number;
} {
  return {
    d: a * 2,
    e: b * 2,
    f: c * 2
  };
}

Deviating from that by adding a bunch of new stuff to learn (that doesn't have as much expressive power) really doesn't seem like a good choice. (Though I do take the point that it would be nice to infer things like data and children when there are also explicitly typed props.)

@gulbanana
Copy link

I don’t know about other Typescript users, but as you say, its interactions with destructuring are bad - in my case this means I very rarely use destructuring in TS code. I’d be a shame for Svelte 5 to make it a mandatory part of how components work.

@ottomated
Copy link
Contributor

Glad to get an official statement on this! I can't say I agree with all your reasoning (neither line nor character count are a great measure of readability or writability), and this does feel like an explicit DX downgrade from svelte 4, but I understand the case for only allowing one option.

I do think it's a shame that this can't be implemented in userland because of this issue sveltejs/language-tools#1567. I also am curious about real-world usage—I expect that the vast majority of components only use one or two props (the case where $props hurts the most.

@Snailedlt
Copy link
Contributor

Snailedlt commented Aug 20, 2024

@Rich-Harris Thanks for the explanation. I agree that having one standard is better than having two, but like someone mentioned already, the DX for $props feels like a significant downgrade from Svelte 4's simple export let prop declaration.

I still don't fully understand why we had to go away from the superior DX of using export let. It's so simple and elegant compared to the $props rune.

Afaik the only downside with export let is that you have to do some weird stuff to get typescript to play along... but I don't understand what the technical reason for that is.
It seems to me that if typescript was fixed for export let it would be superior to $props in every way.
I think if you thoroughly explained why export let was not a good solution, more people would be open to the new $props rune, and perhaps someone could even come up with a more DX friendly alternative which still holds the same benefits.

@Nelhoa
Copy link

Nelhoa commented Aug 20, 2024

It's worth noting that while the example above...

const simple: string = $prop();
const optional: string = $prop('fallback value');

const value: number = $prop.bindable();
const class_: string = $prop().as('class');

const rest: HTMLButtonAttributes = $prop.rest();
...is fewer lines than today's version with Prettier applied...

const {
simple,
optional = 'fallback value',
value = $bindable(),
class: class_,
...rest
}: {
simple: string;
optional?: string;
value: number;
class: string;
} & HTMLButtonAttributes = $props();
...it's actually more characters. When I look at the first one, it takes more effort to understand which non-rest props exist — I need

More characters but way nicer to read IMO.

@david-plugge
Copy link

It all comes down to personal preferences.
To me export let ... looked kinda nice in the beginning, but once need a few more props it Just gets messy, especially with TS (i can not live without TS). And forther more, the word export does not really describe what is going on. We are not exporting anything.
And imo all proposed single prop alternatives in here feel so bulky compared to how its done now.

@Rich-Harris
Copy link
Member

Rich-Harris commented Aug 20, 2024

I think if you thoroughly explained why export let was not a good solution, more people would be open to the new $props rune

Ok fine. export let is bad design for the following reasons:

  • in a normal JS module, export let foo = whatever means that things can import foo from that module, but they cannot change the value of foo — that can only happen inside the module. Yet in a Svelte component, the only purpose of the export keyword in that position is to allow consumers to change the value of foo. In that respect it's a complete reversal of the intent of the keyword, and many people find this unaccountably strange when they first encounter it. There's a reason that the props tutorial includes an apologetic 'this may feel a little weird at first' aside — it's a very confusing piece of syntax. Those of you in this thread are the people who were able to overlook that, but not everyone can. Meanwhile, newcomers to Svelte will be able to go 'oh, I see, it's like destructured function arguments. This is familiar from other frameworks I've used'

  • export let and export const mean totally different things (export const always creates an 'accessor', export let only does if the compiler options include the arcane accessors: true option). This necessitated a very confusing warning when a prop is unused, because we can't be sure that it's not just supposed to be an accessor. In Svelte 5, we eliminate this confusion, and no longer need the weird accessors: true compiler option. That's a bunch less stuff to learn

  • if you need to rename a prop, you have to separate the export from the declaration and do ridiculous stuff like this:

    let klass;
    export { klass as class }
  • in addition to that constraint, prop names must be valid identifiers. You can't do <Child foo-bar={42} />, for example. In Svelte 5, you can. There's a valid debate over what prop names should be allowed, but it's much better to make the distinction based on what makes sense, rather than because of the arbitrary constraints of a flawed design

  • Even when the declaration and the export don't need to be separated, I've seen cases where export let declarations were scattered messily around a component, rather than being in one place. That's corrosive to understanding, and it's impossible when you can only have a single declaration

  • In Svelte 5 you can easily do things like discriminated unions...

    <!-- Input.svelte -->
    <script lang="ts">
      type Props = { type: 'text'; value: string } | { type: 'number'; value: number };
      let { type, value }: Props = $props();
    </script>
    <script lang="ts">
      import Input from './Input.svelte';
    </script>
    
    <!-- cool -->
    <Input type="text" value="a string" />
    <Input type="number" value={42} />
    
    <!-- type error -->
    <Input type="number" value="a string" />
    <Input type="text" value={42} />

    ...which are completely impossible in Svelte 4. Maybe that doesn't feel like a significant limitation day-to-day, but it impacts the quality of component libraries in the ecosystem

  • there's no way to declare rest props, so we have the sinfully ugly $$restProps magic variable, and no way to specify its types. (Technically that's not true, there's interface $$Props, but a) be honest, how many of you knew about it, and b) that's just piling ugliness on top of ugliness)

  • export let offered no way to differentiate between bindable and non-bindable props

You might say that each of these issues is a relatively minor or infrequent concern, and I would believe you. But when you add all these things up, and especially if you approach it from the perspective of someone new to the framework, it becomes clear that it's a bad design.

Just to provide some context: we have been actively discussing $prop() versus $props() on the team for over a year, since well before this issue was created. We have explored every idea in this thread and then some, and consistently concluded that on balance, when considering the totality of use cases and the ways in which we will need to teach them, and based on thorough surveys of how other projects approach this topic, the current design is the best one we've seen yet.

As such, I'm going to close this issue.

@Rich-Harris Rich-Harris closed this as not planned Won't fix, can't repro, duplicate, stale Aug 20, 2024
@mon-jai
Copy link

mon-jai commented Aug 20, 2024

In that respect it's a complete reversal of the intent of the keyword, and many people find this unaccountably strange when they first encounter it.

Hi @Rich-Harris, would you kindly consider dropping export const and export function too, and replacing them with a rune?

They also have entirely different semantics compared to their vanilla JavaScript counterparts, and feel weird as well: #12778 (comment)

@Snailedlt
Copy link
Contributor

Thank you for the explanation, that clarified a few things!
To be honest I thought there would be more commonly encountered issues in the list of reasons export let is bad, so seing this makes me even more sad that we're moving away from it.

  • Imo the point of "this is not how JavaScript usually works" is by enlarge a bad one. Svelte doesn't need to be the exact same as JavaScript, in fact I think a lot of people like that Svelte makes things easier than plain JavaScript. Some of this ease of use can be attributed to less cluttered and shorter syntax (like export let), which may not follow JavaScript conventions or rules, but still with a short explanation are perfectly understandable and reasonable considering the improved DX they offer.

  • I haven't used export const much, so I won't comment on it. I'll just assume you're right on this one.

  • Renaming props is now a bit less ugly I agree, but when do we actually NEED or even WANT to rename props? Not often if ever, at least that's what I think.

  • I agree on your valid identifiers point, though I don't think it should be given much weight, since the most common naming conventions are already possible.

  • Even when the declaration and the export don't need to be separated, I've seen cases where export let declarations were scattered messily around a component, rather than being in one place. That's corrosive to understanding, and it's impossible when you can only have a single declaration.

    I agree, but should we make the basecase (that is, 1-3 props with typescript per component) harder to type to prevent someone from making an active decision not to follow conventions? Again, I just think the tradeoff is not worth it. The new $props syntax is significantly harder to type due to the amount of special characters it uses, and more subjectively it's also "uglier".

  • $$restProps is ugly I agree, but it's a better tradeoff than having the default syntax for everything be cluttered, which imo is what the new $props syntax is.

  • In Svelte 5 you can easily do things like discriminated unions...

    This is great!

  • export let offered no way to differentiate between bindable and non-bindable props

    I realize this is ignorant of me, but I'm here to get a better understanding, so I'll just ask. In which cases do we want or need to make a distinction between bindable and non-bindable? When is it useful, and what patterns does it allow?

All that said I'm glad to see discriminated unions getting some love, as it is something I've been struggling with personally (though not in Svelte).

Sidenote:
I wish there would be more discussions with users of Svelte before things are decided (polls on GitHub and Discord would be a good start). Maybe some of the community members could have come up with a more elegant way of fixing props without making it so ugly and tedious to write.

@FeldrinH
Copy link

Sidenote: I wish there would be more discussions with users of Svelte before things are decided (polls on GitHub and Discord would be a good start). Maybe some of the community members could have come up with a more elegant way of fixing props without making it so ugly and tedious to write.

I think this is a very valid point. It is great that the Svelte team has been discussing this issue internally and has considered the tradeoffs carefully, but I for one would really appreciate if these discussion were more visible to the community. I have been following this issue for months, but the response from Rich Harris 2 days ago is the first time that I have seen any acknowledgement that the Svelte team is even aware of this issue.

@dummdidumm
Copy link
Member

That is not true, I have commented two times before that. But yes, we could have articulated our reasoning more clearly earlier.

@FeldrinH
Copy link

That is not true, I have commented two times before that. But yes, we could have articulated our reasoning more clearly earlier.

That's fair, I had missed those two comments. Though there does seem to be a pretty big gap in communication between "We did not add it yet for a couple of reasons." from September last year to "We have no intention of changing how props work. The lack of a singular $prop rune isn't an oversight, it's the result of many, many hours of careful deliberation."

@Evertt
Copy link

Evertt commented Aug 21, 2024

I want to give my 2 cents about the wish that the Svelte team would involve the community more in their (normally internal) discussions about how to design the Svelte language, especially before they actually make a final decision.

A couple years ago I probably would've agreed. I too deeply appreciate and care about syntax that is easy on the eyes, contains as little noise possible, so that there are minimal distractions when trying to read code and understand what it does.

(click to read my explanation about how I get to my conclusion)

In the past, I've made feature requests for various libraries, believing I had solutions to improve their API so that the consumers of their library would automatically be able to write code that was very easy to read and understand. However, contributors often explained how my suggestions would create more problems than they solved.

These experiences taught me that my understanding of complex libraries and frameworks is limited compared to long-time contributors. What seems suboptimal for simple use cases often proves invaluable in more complex scenarios.

I've noticed similar patterns in this thread. Some confidently propose syntax changes, while team members like @Rich-Harris or @dummdidumm expertly identify multiple issues with these suggestions. This consistently demonstrates the Svelte team's deeper understanding of the language and the far-reaching implications of design choices.

These interactions are both humbling and enlightening, reinforcing the value of the team's expertise and experience.

(Btw, yes I used AI to write that, because what I had written here originally was terribly long-winded / bloated.)

That's why I don't think that the Svelte Team should always involve the community in their decision-making process. The team's expertise likely surpasses most suggestions from the community, and involving the community in every decision would significantly slow down development. The team's time is better spent making informed decisions and writing code for Svelte, rather than managing extensive community input for each choice.

@FeldrinH
Copy link

FeldrinH commented Aug 22, 2024

That's why I don't think that the Svelte Team should always involve the community in their decision-making process. The team's expertise likely surpasses most suggestions from the community, and involving the community in every decision would significantly slow down development. The team's time is better spent making informed decisions and writing code for Svelte, rather than managing extensive community input for each choice.

I don't really disagree. I can certainly see the value in having a more streamlined decision making process that doesn't involve the community in every step. However, I do wish that the Svelte team would communicate their internal discussions and decisions more often and more publicly, so that the broader community could see where they stand on contentious issues and could consequently spend less time and energy on discussions and proposals that ultimately won't influence the direction of Svelte.

@Evertt
Copy link

Evertt commented Aug 22, 2024

I don't really disagree. [...] However, I do wish that the Svelte team would communicate their internal discussions and decisions more often and more publicly, so that the broader community could see where they stand on contentious issues and could consequently spend less time and energy on discussions and proposals that ultimately won't influence the direction of Svelte.

Yeah, sounds fair. Though that would really only work if the community can actually accept that when the Svelte team communicates about a design decision they've made including how they got to their final decision, it's really just an update, and not an invitation to further discuss the topic.

Or maybe they could just mention in their post something like:

This is just to let you guys know what decisions we've made and why. We have considered many alternatives and weighed all the pros and cons and we are confident that we chose the option that strikes the best balance between value added and the limitations it has or the compromises it required.

Therefor we won't spend any time responding to comments that asks us to consider an alternative suggestion that you just thought of, because we've most likely already considered that alternative".

@Snailedlt
Copy link
Contributor

Snailedlt commented Aug 22, 2024

Yeah, sounds fair. Though that would really only work if the community can actually accept that when the Svelte team communicates about a design decision they've made including how they got to their final decision, it's really just an update, and not an invitation to further discuss the topic.

I see no issue with saying "this is our decision, and we just want you to inform you about it".

I just want them to be able to justify their decisions (like Rich did in his recent comments), and let the community know that this is what they think is the best for Svelte moving forward.

The community will comment on this given the chance, and the Svelte team is free to ignore it, discuss it, consider it, or do whatever else they'd like with the feedback they're given. Either way I still think it's valuable to have the decisionmaking process be public, and give the community a place to voice their opinion about recent changes.
I think it's both useful for the Svelte team, the Svelte framework and for the community as a whole. Information is very rarely hurtful, so long as it isn't false.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests