fix(reactivity): should not track effect in onScopeDispose

[rudyxu1102]

Fixes #6538

[skirtles-code]

I'm not very familiar with this part of the codebase, but I did have a couple of thoughts...

1. Is there a reason to call pauseTracking() and resetTracking() inside each cleanup function, rather than doing it once in stop, either side of the for loop that invokes these functions?
2. Do we need to handle error cases here? I notice that run uses try/finally for something similar. I'm thinking maybe resetTracking() should be in a finally block?

[rudyxu1102]

Updated, thanks for your reivew.

[skirtles-code]

This isn't what I had in mind when I suggested handling the error cases. I just meant putting the resetTracking() call in a finally block to ensure it always gets called. Perhaps there are more complete ways to handle errors, but that feels like it's out of scope for the tracking changes being proposed in this PR.

There are a couple of problems with the error handling being proposed here:

1. This error message would be included in production builds, impacting bundle size. Error messages generally aren't included in production builds.
2. Catching the errors will prevent them being handled by app.config.errorHandler.

For example, consider this Playground:

• Playground - main

Notice that clicking the button triggers the errorHandler. Contrast that with how it behaves with this PR:

• Playground - this PR

I don't think this is desirable.

[edison1105]

I agree with @skirtles-code.

[github-actions]

Size Report

Bundles

File	Size	Gzip	Brotli
runtime-dom.global.prod.js	99.2 kB (+105 B)	37.6 kB (+59 B)	33.8 kB (+45 B)
vue.global.prod.js	157 kB (+105 B)	57.4 kB (+56 B)	51 kB (-44 B)

Usages

Name	Size	Gzip	Brotli
createApp	54.5 kB (+10 B)	21.1 kB (+4 B)	19.3 kB (+15 B)
createSSRApp	58.5 kB (+10 B)	22.8 kB (+5 B)	20.7 kB (+3 B)
defineCustomElement	59.1 kB (+10 B)	22.6 kB (+5 B)	20.6 kB (+37 B)
overall	68.1 kB (+10 B)	26.2 kB (+5 B)	23.8 kB (-2 B)

[pkg-pr-new]

commit: 0034360

@vue/compiler-core

pnpm add https://pkg.pr.new/@vue/compiler-core@7265

@vue/compiler-dom

pnpm add https://pkg.pr.new/@vue/compiler-dom@7265

@vue/compiler-sfc

pnpm add https://pkg.pr.new/@vue/compiler-sfc@7265

@vue/compiler-ssr

pnpm add https://pkg.pr.new/@vue/compiler-ssr@7265

@vue/reactivity

pnpm add https://pkg.pr.new/@vue/reactivity@7265

@vue/runtime-core

pnpm add https://pkg.pr.new/@vue/runtime-core@7265

@vue/runtime-dom

pnpm add https://pkg.pr.new/@vue/runtime-dom@7265

@vue/server-renderer

pnpm add https://pkg.pr.new/@vue/server-renderer@7265

@vue/shared

pnpm add https://pkg.pr.new/@vue/shared@7265

vue

pnpm add https://pkg.pr.new/vue@7265

@vue/compat

pnpm add https://pkg.pr.new/@vue/compat@7265

Open in Stackblitz

[edison1105]

the changes should like below:

      stop(fromParent?: boolean): void {
        if (this._active) {
      	  try{
      	    pauseTracking()
      	    //...
      	  } finally {
      	    resetTracking()
      	  }
      	}
      }