-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
breaking: add $props.bindable() #10804
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
Conversation
|
@@ -0,0 +1,4 @@ | |||
<script> | |||
let { x } = $props(); | |||
x.y = 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yay or nay for throwing a compiler error in this case? The argument against it would be setters on a class, but I'm wondering how big of a deal it really is to make the prop bindable in that case, since this will in the vast majority of cases guide people towards the right outcome. Another argument for keeping it is that besides the runtime error telling you that you can't bind:
to a prop when it's not declared with $props.bindable()
is the only runtime error you get. When mutating a prop, you'll only get the "you better not mutate this" warning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ugh .. if we keep the compiler error then we need to have a SvelteKit release veeery soon after it, because, as you can see, the site won't build right now since we're getting a compiler error for our generated root.svelte
.
closes #10768
closes #10711
Before submitting the PR, please make sure you do the following
feat:
,fix:
,chore:
, ordocs:
.Tests and linting
pnpm test
and lint the project withpnpm lint