fix: correctly teardown bind:this with $state.frozen

[paoloricciuti]

Svelte 5 rewrite

Closes #12287

The teardown was incorrectly fired because if the user uses state.frozen there's no [STATE_SYMBOL].v but the whole bound value is the "original target".

Note that this only happen with dev: false because in dev the methods $on, $set and $destroy are added to throw errors and the problem was specifically when the bound value was undefined (the component was not exporting anything).

I might be on the wrong path (i'm just wondering why this was a problem only for undefined) but it seems to make sense.

P.s. there's a completely unrelated failing tests, maybe is on main?

Please note that the Svelte codebase is currently being rewritten for Svelte 5. Changes should target Svelte 5, which lives on the default branch (main).

If your PR concerns Svelte 4 (including updates to svelte.dev.docs), please ensure the base branch is svelte-4 and not main.

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: 506b0ea

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

[dummdidumm]

I think this hides a different bug: A component should always return an object, even if it's empty, so that bind:this won't be falsy. This got lost in a refactoring because there was no test for it. So the proper fix would be to add something to transform-client.js, like I did in my commit.

[paoloricciuti]

I think this hides a different bug: A component should always return an object, even if it's empty, so that bind:this won't be falsy. This got lost in a refactoring because there was no test for it. So the proper fix would be to add something to transform-client.js, like I did in my commit.

Yeah I had the impression that something was wrong anyway and wasn't aware that the component should always return an object even if it didn't export anything.

However it still seemed strange not checking for state frozen in that function, do you think this could lead to other bugs?

[trueadm]

However it still seemed strange not checking for state frozen in that function, do you think this could lead to other bugs?

I don't think we should be treating frozen state differently

[paoloricciuti]

However it still seemed strange not checking for state frozen in that function, do you think this could lead to other bugs?

I don't think we should be treating frozen state differently

Gotcha...sorry for the small mess-up i hope the exploration was at least useful to find the right fix quicker :)