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

test: add initial component tests #923

Merged
merged 14 commits into from
Nov 17, 2023

Conversation

jrutila
Copy link
Contributor

@jrutila jrutila commented Nov 6, 2023

πŸ”— Linked issue

#546

❓ Type of change

  • πŸ“– Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)
  • Tests

πŸ“š Description

Adds component unit tests using nuxt-vitest. There is a general component-render function that can be used to render the function. The rendered outcome is saved to the vitest snapshots, which is a quite nice way to do it in this kind of module to prevent accidental changes. You can either give the options for the component in code or write direct HTML.

One future development idea is to source the docs for component examples and use those as additional test cases as we only need the HTML of the component.

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

Copy link

vercel bot commented Nov 6, 2023

The latest updates on your projects. Learn more about Vercel for Git β†—οΈŽ

Name Status Preview Updated (UTC)
ui βœ… Ready (Inspect) Visit Preview Nov 16, 2023 5:30pm

@benjamincanac
Copy link
Member

Have you seen the TypeError: Cannot read properties of undefined (reading 'preference') errors in https://github.com/nuxt/ui/actions/runs/6775814945/job/18415950066?pr=923? I guess we need to find a way to disable @nuxtjs/color-mode during tests.

@jrutila
Copy link
Contributor Author

jrutila commented Nov 7, 2023

Have you seen the TypeError: Cannot read properties of undefined (reading 'preference') errors

Yes, I saw them. It is because color-mode tries to access a property inside window. I have yet to figure out if I could mock that property before the test run. On the other hand the tests still work even with the error.

Copy link
Member

Do you want to merge this as is?

@jrutila
Copy link
Contributor Author

jrutila commented Nov 7, 2023

I tried to inject to the window, but in vain. It should be possible to do through globalThis. Maybe let's merge like this and fix that error later.

@danielroe
Copy link
Member

danielroe commented Nov 10, 2023

Yes, we need to fix this in color mode module. Here's a patch I made in my own website to handle a different issue with server components:

https://github.com/danielroe/roe.dev/blob/main/patches/%40nuxtjs__color-mode%403.3.0.patch

But we need a PR to the module, which I'll do (or someone else if you get to it first).

@benjamincanac
Copy link
Member

Thanks @danielroe! Seems the latest version of @nuxtjs/color-mode fixes the issue 😊

@benjamincanac
Copy link
Member

@jrutila There is still a warn remaining: [Vue warn]: Avoid app logic that relies on enumerating keys on a component instance. The keys will be empty in production mode to avoid performance overhead., would you know where does this come from?

@danielroe
Copy link
Member

It comes from a recent pr to Nuxt vitest that we need to look at. But it's safe.

@benjamincanac
Copy link
Member

I have a last question, unlike what's mentioned on the README, this PR does not register the nuxt-vitest module. Is it supposed to be used without it?

@jrutila
Copy link
Contributor Author

jrutila commented Nov 16, 2023

I have a last question, unlike what's mentioned on the README, this PR does not register the nuxt-vitest module. Is it supposed to be used without it?

What do you mean? At least there is nuxt-vitest in the package.json.
It doesn't need to be registered to the module, though, as it is only used to dev/test the module. Module users don't need it.

@benjamincanac benjamincanac merged commit bcc46b8 into nuxt:dev Nov 17, 2023
2 checks passed
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.

3 participants