breaking: always run pre effects synchronously

[Rich-Harris]

Currently doing a deep dive on effects, and found myself stumped as to why we treat $effect.pre(...) this way — given this...

$effect.pre(() => {
  log.push(`Outer Effect Start (${count})`)

  $effect.pre(() => {
    log.push(`Inner Effect (${count})`)
  });

  log.push(`Outer Effect End (${count})`)
});

...and an incrementing count, we log this:

Outer Effect Start (0)
Outer Effect End (0)
Inner Effect (0)
---
Outer Effect Start (1)
Outer Effect End (1)
Inner Effect (1)
---
Outer Effect Start (2)
Outer Effect End (2)
Inner Effect (2)

This strikes me as rather strange. After all, if the user wanted Inner Effect to run after the other two logs, they'd just... put the inner effect at the bottom of the outer effect.

Is there some advantage to doing it the other way that I'm not seeing? Seems like it adds complexity and uncertainty for no gain.

[changeset-bot]

🦋 Changeset detected

Latest commit: 3f39ac7

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]

Spelunking through the git history, I find this comment from @dummdidumm (from a separate, private repo)...

The inner $effect.pre fires twice, once before the outer one and again after it

...followed by this on a PR from @trueadm that fixes that issue:

Nested pre-effects were incorrectly being made sync when they should be queued like normal effects

The double-firing issue doesn't happen with this PR, and we have a regression test, so I'm not worried that it might start to do that. So the unresolved question is what the rationale is for 'they should be queued like normal effects'

[trueadm]

Yeah let's go for it.