Skip to content

Conversation

Rich-Harris
Copy link
Member

this doesn't do anything

Copy link
Contributor

github-actions bot commented Jul 7, 2025

Playground

pnpm add https://pkg.pr.new/svelte@16317

Copy link
Member

@dummdidumm dummdidumm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...yet. in the async branch it's needed because there it's only set when it's an effect, not when it's a derived. So we should just keep it

@Rich-Harris
Copy link
Member Author

I think that would be better expressed inside push_reaction_value:

-if (active_reaction !== null && active_reaction.f & EFFECT_IS_UPDATING) {
+if (active_reaction !== null && (!async_mode_flag || (active_reaction.f & DERIVED) !== 0)) {
  if (source_ownership === null) {
    source_ownership = { reaction: active_reaction, sources: [value] };
  } else {
    source_ownership.sources.push(value);
  }
}

To me that's a lot clearer than an EFFECT_IS_UPDATING flag (which aside from any async mode nuance suggests 'the effect is updating' rather than 'the thing that is updating is an effect'), and it'll be more obvious what change needs to happen when we remove async_mode_flag. Most importantly, it saves us a bitwise flag (we're running out of them).

But I can make that change on the async branch instead, so I'll close this

@Rich-Harris Rich-Harris closed this Jul 8, 2025
@dummdidumm dummdidumm deleted the remove-effect-is-updating branch July 8, 2025 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants