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(runtime-core): ensure jobs are in the correct order #7622

Closed
wants to merge 1 commit into from

Conversation

edison1105
Copy link
Member

@edison1105 edison1105 commented Feb 2, 2023

fix #7576

chore: improve code

chore: improve code

chore: improve code

chore: improve code
@LinusBorg LinusBorg added scope: reactivity 🔨 p3-minor-bug Priority 3: this fixes a bug, but is an edge case that only affects very specific usage. labels Feb 14, 2023
@skirtles-code
Copy link
Contributor

I don't think it should matter whether the new job is pre: true or not, the insertion point should be the same either way. I think we only need to check the pre flag on jobs that are already in the queue.

I believe this change would introduce a regression. The middle-- part seems unnecessary, and can lead to the insertion point being before the flushIndex:

SFC Playground - regression

Notice how the component doesn't re-render when the button is clicked, because the rendering job is inserted too early in the queue.

Here's that same example with the current main:

SFC Playground - working

I've opened a separate PR, #7748, where I've proposed an alternative fix for the original problem.

@edison1105
Copy link
Member Author

@skirtles-code
Thank you for the review and your point is correct.
Close this one.

@edison1105 edison1105 closed this Feb 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 p3-minor-bug Priority 3: this fixes a bug, but is an edge case that only affects very specific usage. scope: reactivity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Watchers no longer fire in the order they're defined from v3.2.38
3 participants