Conversation
|
dummdidumm
left a comment
There was a problem hiding this comment.
This means we keep around an array of sources for longer vs one reaction previously. Hard to say what keeps more memory, I assume it's largely the same, so 👍
|
That's true... an alternative would be to prevent push_reaction_sources from running if the child source is being created after the initial reaction. I tried to make that work but it was finicky. If I'd realized we were hanging onto the array for longer with this change I'd have kept plugging away at it |
🦋 Changeset detectedLatest commit: 85e7439 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 |
|
Changed the implementation so that |
I have a feeling we could make this simpler still, by comparing the
update_versionwith the value at the time when the signal was created, but I haven't yet figured out an approach that doesn't result in an additional property on the signal (I thought we could userv, but it makes it harder to avoid duplicate dependencies).In the meantime, we don't need to create an object with a
reactionproperty in addition to the sources; as long as we updatesource_ownership(here renamed tocurrent_sources) when we need to lazily create sources insideproxy.js. Previously we were usingset_active_reactionas a way to achieve the same outcome, but in a more roundabout way (normally that function is used so that the effect tree is constructed correctly, but that doesn't apply here).Before submitting the PR, please make sure you do the following
feat:,fix:,chore:, ordocs:.packages/svelte/src, add a changeset (npx changeset).Tests and linting
pnpm testand lint the project withpnpm lint