feat: make fallback prop values readonly

[Rich-Harris]

closes #9763

Before submitting the PR, please make sure you do the following

• It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• Prefix your PR title with feat:, fix:, chore:, or docs:.
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.

Tests and linting

• Run the tests with pnpm test and lint the project with pnpm lint

[changeset-bot]

🦋 Changeset detected

Latest commit: 23064b1

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
svelte	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
svelte-5-preview	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Dec 5, 2023 9:02pm

[trueadm]

I'm starting to think it might make more sense to have two versions of this – one for runes and one for legacy. I'm really unhappy that we have sync_effect as it's an anti-pattern, so if we can somehow get to a point where we kill that in runes, it enables us to make forking of effects work. With sync effects that happen during the notification phase, that's pretty much impossible as it means effects fire and potentially invalidate another effect tree, making forking impossible.

[colelawrence]

@Rich-Harris, FYI this breaks my previously working code where I am intentionally passing settable values down into my component as normal setter functions, and based on the docs and my prior knowledge of Svelte. I cannot understand how to fix this new error.
e.g.

<script lang="ts">
  import type { ITodo } from "./createApp.svelte";

  // ...
  const { todo } = $props<{
    todo: ITodo;
  }>();
</script>
...
<input
      type="text"
      class="grow"
      bind:value={todo.text}
      bind:this={todo.htmlInputElement}>

But, this will break with these changes.

So, there are two things: I don't understand how to set this up in a different way.
Here is an example of where I use bind:this={prop.htmlElt} on a passed in prop
and here is the main business logic written in a sort of MVVM pattern.

Separately, this seems like an amazing opportunity to add something to the error message that mentions the name being accessed, since Webkit/JavaScriptCore throws away the original stack trace of the error in report_error, making this incredibly challenging to track down

[colelawrence]

Am I correct to understand that the fix is to simply mark the users with bind like <AddTodoForm bind:addTodo={app.addTodo} /> and <TodoItem bind:todo={todo}/>? If so, I think we have enough information to spell this entire solution out as part of the error, so the error could be

Props cannot be mutated, unless used with `bind:`. Use `bind:todo={...}` to make properties on this prop mutable.

[dummdidumm]

Can you provide a REPL link with code that you think is broken now? In general, with this change, you have to use bind:.. for any state that you pass and modify somewhere in the component tree below, unless you're using getters/setters/state properties on a class, because those are not wrapped with the readonly-checker. The idea is to prevent manipulating the prop unless the parent allows it, or you pass in an object that is not a POJO and is therefore modifyable through other means (functions, setters).

[colelawrence]

@dummdidumm - I was able to make my code work in this way: colelawrence/here-now@073acd5#diff-8f574d4bbd38fcef4678b28158a9d7b504178716321ea7fc7f8a97ea2fec0e38R78

I think the idea is fine given that this all works together, but I would strongly suggest adding a clearer runtime error message to help the developer find and fix the issue.

Props cannot be mutated, unless used with `bind:`. Use `bind:todo={...}` to make the properties like `htmlInputElement` settable.

Or something similar.

Here's a link to the logic that no longer works. I'm not sure if it's a result of this exact PR or another recent PR related to this.

https://svelte-5-preview.vercel.app/#H4sIAAAAAAAAE42SQU7DMBBFrzKyKjUVVQrbNKmEegzConGcYkhsy54gIStLdhwBLteT4MRxSAuV2Djx_P9m7PFYUvGaGZI8WCIODSMJuVeKrAm-qX5jXlmNzO2NbDXtI6mhmivc5QKAN0pqhP0Tr0uotGxgGW-GXezB5TYXOVatoMilAKrZAdletgKZjlZgezXHmiHQPggZLAw6S3S78pJm2GoRjDkeg_WHntkGZRvC3Tr8mYlqVXkGhrIuPmH-Z_j0i-hdwiAclHLOEab-EsnlnX5RFkYrdI52OXKRbqYe5uL0-Q6yeGYUs2BMfUPHqB3DHWwckBbaraevjyC7jPEVcCadwa5iwUWZXM8wly_TuGFoZMkrzkqSoG5Zt55mZ6D_Nz3-2e142KE5C6WlMtFqe9GitGgR3fhIQWtOXzLrHj_b-ZfwuD8f3GRwtw2tlzWLa3mM5o4-ddcN1YdUJgE717u-sq_2xz0fu2-ndzwQLwMAAA==

[dummdidumm]

Thanks for the reproducible - this should work, it's a bug that it doesn't. Will open a PR to fix this.