Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: beforeUpdate called twice with bound reference #6858

Conversation

RaiVaibhav
Copy link
Contributor

Fixes: #6016
Fixes: #3290

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

  • Run the tests with npm test and lint the project with npm run lint

@RaiVaibhav
Copy link
Contributor Author

This PR doesn't touch on the cases related to reactivity, please check the comment #6685 (comment)

src/runtime/internal/scheduler.ts Outdated Show resolved Hide resolved
src/runtime/internal/scheduler.ts Outdated Show resolved Hide resolved
@RaiVaibhav RaiVaibhav force-pushed the fix/before-update-getting-called-twice branch from ec6a9ac to fa7b1d4 Compare November 9, 2021 18:08
@bluwy
Copy link
Member

bluwy commented Nov 10, 2021

Thanks for the extra refactors! We'll have to wait and see if the other maintainers have an opinion on this update strategy.

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.

The changes makes sense to me 👍

src/runtime/internal/scheduler.ts Outdated Show resolved Hide resolved
@RaiVaibhav
Copy link
Contributor Author

@rmunn is this PR still valid?

@benmccann
Copy link
Member

@RaiVaibhav if you don't mind rebasing this PR, we hope to review the outstanding PRs in the coming weeks

@RaiVaibhav
Copy link
Contributor Author

RaiVaibhav commented Feb 23, 2023 via email

@benmccann
Copy link
Member

You can update it a few days from now. That will be fine

@vercel
Copy link

vercel bot commented Mar 14, 2023

@dummdidumm is attempting to deploy a commit to the Svelte Team on Vercel.

A member of the Team first needs to authorize it.

@benmccann benmccann changed the title Fix: beforeUpdate called twice with bound reference fix: beforeUpdate called twice with bound reference Mar 14, 2023
@dummdidumm dummdidumm added this to the one day milestone Apr 3, 2023
@RaiVaibhav
Copy link
Contributor Author

Kindly let me know if I can still help

@dummdidumm
Copy link
Member

Thank you for the offer! Svelte 5 deprecates beforeUpdate/afterUpdate in favor of $effect.pre/$effect, which behave correctly. Therefore closing this PR.

@dummdidumm dummdidumm closed this Apr 9, 2024
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.

beforeUpdate called twice with bound reference Unexpected Update Calls
4 participants