fix: address runtime effect issues #9417
Conversation
🦋 Changeset detectedLatest commit: 8ea82ee The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| $effect(() => { | ||
| let double = $derived(count * 2) | ||
|
|
||
| log.push('init ' + double); |
There was a problem hiding this comment.
I think this test can move into tests/signals - or do we need something Svelte-render-specific here? Seems purely related to the signals runtime.
There was a problem hiding this comment.
It matches the other tests inside the directory that checks logging order? It seems related to Svelte runtime too.
There was a problem hiding this comment.
Which other tests are you talking about? Maybe those other tests need to move into tests/signals, too 😄
There was a problem hiding this comment.
I tried, but I now get test failures accessing component. Let's leave them for now and address in another PR.
There was a problem hiding this comment.
what test failures about accessing component? When you try to turn this test into a test in signals then it fails or what do you mean exactly?
There was a problem hiding this comment.
The test suite setup is different I guess?
There was a problem hiding this comment.
So I've done some digging and ultimately I don't think these are signals tests and they shouldn't be coupled with being so. Signal tests should be agnostic of Svelte's components. However, these cases come from bug reports where users have created components that demonstrate ordering issues or timing issues that are relevant to their components. This is important as the signals tests also demonstrate unowned signals – which mean they are independent of components too, not to mention how this all fits into equality.
Let's merge this PR and create a separate issue on how we can unify the semantics around signals - vs - component state semantics, as I think there needs to be a distinction and having one is important. For example, our current signals tests are all wrapped with a render effect, which is likely wrong too. They shouldn't be, signals should be agnostic of effects.
There was a problem hiding this comment.
The idea of wrapping them is to simulate this being run inside a component context, which is why I was so confused why it wouldn't work when having them in that test suite
This PR addresses several issues relating to the runtime:
Symbol()when component unmounts #9409.$effect.pretriggers an error when called outside of having an active effect parent. Fixes Svelte 5:$effect.pre()callable without error outside of component initialisation #9410.__properties in operations, which were pointing to the wrong prototype.Before submitting the PR, please make sure you do the following
feat:,fix:,chore:, ordocs:.Tests and linting
pnpm testand lint the project withpnpm lint