fix: ensure dynamic event handlers are wrapped in a derived

[trueadm]

Addresses the issue mentioned in #12519 that is for Svelte 5. Fixes #12520.

[changeset-bot]

🦋 Changeset detected

Latest commit: cb728f0

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]

We probably only want to do this if there's a CallExpression, no? That's the standard we apply elsewhere

[Rich-Harris]

i.e. here...

<script>
  let count = $state(0);

  const handlers = {
    increment: () => count += 1
  };
</script>

<button onclick={handlers.increment}>
  clicks: {count}
</button>

var button = root();
-var event_handler = $.derived(() => handlers.increment);

button.__click = function (...$$args) {
- return $.get(event_handler)?.apply(this, $$args);
+ return handlers.increment?.apply(this, $$args);
};

[trueadm]

We probably only want to do this if there's a CallExpression, no? That's the standard we apply elsewhere

Ah yeah, I don't need that other change anymore. Good spot!

[Rich-Harris]

We're still creating a derived for conditional/logical expressions, which we don't do in other scenarios. Worth avoiding?

[trueadm]

@Rich-Harris I can split it out so we only create dynamic deriveds for call expressions, but we still need to keep dynamic handlers. I guess we would also need to be more thorough as the conditional could contain a call expression.

[Rich-Harris]

We have a mechanism for this that we use everywhere else we face the same problem — contains_call_expression — which handles the 'call expression inside a conditional expression' case. Any reason we can't use it here?

[trueadm]

We have a mechanism for this that we use everywhere else we face the same problem — contains_call_expression — which handles the 'call expression inside a conditional expression' case. Any reason we can't use it here?

That's what I tried originally, unfortunately, it doesn't seem like it's available? It's not on the handler node aka node.expression.

[trueadm]

Ah figured it out!