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): fix immediate callback not triggered bug for multisource #5467

Closed
wants to merge 2 commits into from

Conversation

ioslh
Copy link
Contributor

@ioslh ioslh commented Feb 21, 2022

Fix this bug:

  1. when watching multi-source with { immediate: true } option
  2. and the init value of the ref value is undefined

the callback won't get called immediately as expected.

See reproduce buggy code detailed below( or sfc playground link):

// important to reproduce: set init value to Ref<undefined>
const msg = ref()

watch(msg, () => {
  console.log('this will be called immediately as expected')
}, { immediate: true })
  
watch([msg], () => {
  console.log("but this won't")
}, { immediate: true })

When determining if callback should be called immediately for the first time ( relative code here ) , the hasChanged get false for multi source(newValue is[undefined] and oldValue is [], [undefined][0]===[][0] ) while get true for single source, which cause different behaviors.

This PR fix it by compare the reference valueINITIAL_WATCHER_VALUE to newValue for multi source, just like what single source case do.
And when we call callback, pass undefined for single source and [] for multi source for the first time, keep this correct behavior.

@netlify
Copy link

netlify bot commented Feb 21, 2022

✔️ Deploy Preview for vuejs-coverage ready!

🔨 Explore the source changes: b8cd388

🔍 Inspect the deploy log: https://app.netlify.com/sites/vuejs-coverage/deploys/6213be60f855e90008a9b74d

😎 Browse the preview: https://deploy-preview-5467--vuejs-coverage.netlify.app

@netlify
Copy link

netlify bot commented Feb 21, 2022

✔️ Deploy Preview for vue-sfc-playground ready!

🔨 Explore the source changes: b8cd388

🔍 Inspect the deploy log: https://app.netlify.com/sites/vue-sfc-playground/deploys/6213be60491be400078dd9b2

😎 Browse the preview: https://deploy-preview-5467--vue-sfc-playground.netlify.app/

@netlify
Copy link

netlify bot commented Feb 21, 2022

✔️ Deploy Preview for vue-next-template-explorer ready!

🔨 Explore the source changes: b8cd388

🔍 Inspect the deploy log: https://app.netlify.com/sites/vue-next-template-explorer/deploys/6213be605a352f00073229d2

😎 Browse the preview: https://deploy-preview-5467--vue-next-template-explorer.netlify.app

@LinusBorg
Copy link
Member

I also took a stab at this in #5033

@ioslh ioslh closed this Feb 28, 2023
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.

None yet

2 participants