Differentiate script setup from option api component with setup

[renatodeleao]

After a lot of discussion at #1869 and failed attempts to reproduce the issue I finally made the error happen and propose a fix. It's very ugly, but the tests are green and make more VTU@2 "customers" happy (at least one customer :) )

The patch

Using yet another vue internal property __isScriptSetup¹² to differentiate <script setup> components from Option API components with setup() hook since both fall in the hasSetupState block. The latter can be patched regularly without triggering Proxy trap set error that lead to the 2.2.3 patch in the first place (#1861), and only when patched like that, it actually works in all possible scenarios.

The only reason for this to be acceptable code is because the previous implementation also relied on devtoolsRawSetupState which is also considered internal property.

Before

test-utils/src/mount.ts

Lines 481 to 485 in a48712d

	// we need to differentiate components that are or not not `script setup`
	// otherwise we run into a proxy set error
	// due to https://github.com/vuejs/core/commit/f73925d76a76ee259749b8b48cb68895f539a00f#diff-ea4d1ddabb7e22e17e80ada458eef70679af4005df2a1a6b73418fec897603ceR404
	// introduced in Vue v3.2.45
	if (hasSetupState(this)) {

After

test-utils/src/mount.ts

Lines 485 to 488 in aec9623

	// Also ensures not to include option API components in this this block
	// since they can also have setup state but need to be patched using
	// the regular method.
	if (hasSetupState(this) && this.$.setupState.__isScriptSetup) {

Notes for the reviewer

• I only tested the code with the current [email protected] of the codebase.
• If we to keep the git history clean, feel free to rebase and squash both commits, I've kept them to provide an easy way to reset and QA the PR. git reset HEAD^ --hard && npm run test mocks to see the spec failing.
• If the solution is accepted, it's probably better to just rename the helper hasSetupState to isScriptSetup and move the && check within the helper. We can also check if simply using the __isScriptSetup flag is enough.

Footnotes

1. https://github.com/vuejs/core/blob/0c07f12541061cf10996f998705d7ad36193aa4f/packages/runtime-core/__tests__/apiCreateApp.spec.ts#L517 ↩

2. https://github.com/vuejs/core/blob/8a123ac34fd30fc36ac9e0c252dd345d6d7c7f50/packages/compiler-core/src/options.ts#L121 ↩

[netlify]

✅ Deploy Preview for vue-test-utils-docs ready!

Name	Link
🔨 Latest commit	f509cef
🔍 Latest deploy log	https://app.netlify.com/sites/vue-test-utils-docs/deploys/63b551917b442a00083eb9e8
😎 Deploy Preview	https://deploy-preview-1928--vue-test-utils-docs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[cexbrayat]

Thanks for looking into this @renatodeleao 👍

Your fix looks good to me, I was afraid we would need more cases to make everything work.
As I commented in the review, let's try just using _isScriptSetup and see if that's OK.

[cexbrayat]

I like your suggestion to have a isScriptSetup util function 👍
And maybe this check can just rely on _isScriptSetup and we can remove hasSetupState here.
I think hasSetupState is used elsewhere and is really needed to know if the component has a setup (but not necessarily a script setup). If tests are green, we can cut a release and give it a go.

[cexbrayat]

nit: typo this this

[renatodeleao]

Yup, you're right! I've removed the hasSetupState check from here because it's redundant, but we can't replace hasSetupState with isScriptSetup as they check for different things and some specs around expose fail if we did.

Thanks for the review!

[cexbrayat]

Thanks @renatodeleao , we'll cut a release soon