fix: stack-trace-based readonly validation

[Rich-Harris]

Demo

I'm pretty sure I'll look at this in the morning with abject horror, but I thought the idea was at least interesting enough to share even if this PR is really just a proof of concept.

#10378 proposes removing the readonly validation that prevents components from mutating their props. I think it would be a terrible shame to lose this, and so I've been trying as best I can to come up with a suitable alternative.

Here's a rough description of what's happening here (note that all this only applies to dev mode):

1. add metadata to the transformed component

We wrap each component in these function calls:

+$.push_module(Thing);

export default function Thing($$anchor, $$props) {
  // ...
}

+$.pop_module();

push_module and pop_module capture stack traces, allowing us to determine the boundaries of each .svelte file even if some bundling or other transformation has taken place (as is the case in the preview playground, for example).

2. signals are given 'owners'

When creating a signal, we add the current component function to the set of the signal's owners.

When we pass a signal to a child with a binding...

<Child bind:x={y} />

...we add the child as another owner:

export default function Thing($$anchor, $$props) {
  $.push($$props, true);

  let y = $.source(...);

  // ...

+  {
+    $.pre_effect(() => $.add_owner($.get(y), Child));

      Child(node, {
        get x() {
          return $.get(y);
        },
        set x($$value) {
          $.set(y, $.proxy($$value));
        }
      });
+  }

  // ...
  $.pop();
}

add_owner causes a deep_read of its first argument; any get that happens inside causes Child to be added as an owner.

3. capture stack traces during set

Whenever a set happens, we capture a stack trace. Because we know the boundaries of each component, we can reliably determine whether the change originated inside the parent or child component, even if the actual mutation happens inside a function in a third module.

If the most recent component in the stack trace is not among the signal's owners, then we know it has been mutated illegally.

(Importantly, if the child component mutates state via a callback prop passed from the parent component, the parent will be the most recent component in the stack trace, meaning the mutation is treated as safe regardless of any bindings.)

---

Lots still to do:

• deal with state that doesn't belong to a component (e.g. imported from a shared module)
• handle the case where a chunk of state is reassigned (i.e. foo.bar = {...})
• don't add grandchild as owner if child uses bind: with state that belonged to its parent (and which it does not co-own)
• tests
• make the code less raggedy
• figure out what edge cases exist, and whether there are any blockers to this working

Speaking of edge cases, there definitely are some. A stack trace can tell you which component file was the source of a mutation, but not which component instance, so if you have a case like this...

<Child foo={foo} />
<Child bind:foo={foo} />

...then, thanks to the bind:foo, Child will become an owner of foo meaning that the first <Child> component would be able to mutate it even though it isn't bound. This feels like something we could probably live with. There may be other edge cases that we can't live with.

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: 26455c3

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

[Rich-Harris]

Took me way too long to figure this out, but ownership needs to be a proxy concept rather than a signal concept. Everything fell into place after that realisation. Inheriting ownership now works

[trueadm]

If this is now a proxy concept rather than a signal concept, how does state on classes work with this validation?

[Rich-Harris]

It doesn't, it has the same characteristics as the current proxy-based readonly validation, which we justified on the grounds that if you're passing around a custom class instance you probably do intend for consumers to interact with it.

If we wanted to, we could make it work by augmenting the transformed class declaration to include a similar metadata object, and recurse into it. I didn't do that in this PR though

[trueadm]

Thanks for the response. The code looks good. :)