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: return the correct name when stubbing a script setup component #1783

Merged
merged 4 commits into from Sep 25, 2022

Conversation

hershelh
Copy link
Contributor

@hershelh hershelh commented Sep 24, 2022

When stubbing a component using the <script setup> syntax test-utils can't return its name correctly, so it skips the stub and the stub fails.There's an issue that describes the problem.
This is because getComponentName() function doesn't return its __name property.

@netlify
Copy link

netlify bot commented Sep 24, 2022

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

Name Link
🔨 Latest commit 14bb306
🔍 Latest deploy log https://app.netlify.com/sites/vue-test-utils-docs/deploys/63306bc87924ab0008ba60b2
😎 Deploy Preview https://deploy-preview-1783--vue-test-utils-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

Copy link
Member

@cexbrayat cexbrayat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @joeyhuang0235

Can you add a test that checks this change please?

@hershelh
Copy link
Contributor Author

OK, I'll do it later

@hershelh hershelh closed this Sep 24, 2022
@hershelh hershelh reopened this Sep 24, 2022
@hershelh
Copy link
Contributor Author

hershelh commented Sep 24, 2022

@cexbrayat If I want the test case to run expectedly I have to install unplugin-vue-components and add it to Vitest configiration file like this:
image
It only works on the AutoImportScriptSetup component I specify and does not affect other components. Is it OK?

@cexbrayat
Copy link
Member

I guess we can, if that solves a specific issue for this plugin.

Is it possible to limit the application of this plugin only to the test you are adding?

@hershelh
Copy link
Contributor Author

I think I can't. As long as a component is imported, the plugin will take effect, so I can't limit the scope of this plugin to a test, but only control the target it transforms. So I specified that it can only transform the AutoImportScriptSetup component

@cexbrayat
Copy link
Member

Ok, it should be good enough. Push your test and we'll take a look 👍

Copy link
Member

@cexbrayat cexbrayat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few nits, and we're good to merge 👍

package.json Outdated Show resolved Hide resolved
src/utils/componentName.ts Show resolved Hide resolved
vitest.config.ts Show resolved Hide resolved
@cexbrayat
Copy link
Member

You need to run pnpm i again to update the lockfile, and we should be ready to merge

@hershelh
Copy link
Contributor Author

Ok, sorry I forgot

@cexbrayat cexbrayat merged commit 864e9e2 into vuejs:main Sep 25, 2022
@cexbrayat
Copy link
Member

Awesome, thanks @joeyhuang0235

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