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

Bug: PR 2327 introduces regression in vm #2557

Open
davidnixon opened this issue Nov 26, 2024 · 2 comments
Open

Bug: PR 2327 introduces regression in vm #2557

davidnixon opened this issue Nov 26, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@davidnixon
Copy link

Describe the bug

This change
#2327
broke my test case.

To Reproduce

Pull this PR
carbon-design-system/carbon-components-vue#1662
Run
yarn test src/components/CvAccordion/__tests__/CvAccordion.spec.js

The change in the PR #2327 removes the visibility of componentRef in the vm. It maybe related to the component using defineExpose({ state }); which is not used elsewhere in our components.

Reverting the change on #2327 fixes the test case.

Expected behavior

    const acc = wrapper.findComponent(`.${carbonPrefix}--accordion`);
    expect(Boolean(acc.vm.componentRef)).toBe(true);

Related information:

Additional context

@davidnixon davidnixon added the bug Something isn't working label Nov 26, 2024
@davidnixon davidnixon changed the title Bug: Bug: PR 2327 introduces regression in vm Nov 26, 2024
@cexbrayat
Copy link
Member

Pinging @freakzlike who authored the PR as he may have more insights

@freakzlike
Copy link
Collaborator

I'm not quite sure if I understand correctly, but what I see in your test case, that it's working correctly now and it was wrong previously:

https://github.com/carbon-design-system/carbon-components-vue/blob/main/src/components/CvAccordion/__tests__/CvAccordion.spec.js#L60

<template>
  <cv-accordion @change="handleChange" ref="componentRef">
  // ...
// finds the instance of CvAccordion.vue
const acc = wrapper.findComponent(`.${carbonPrefix}--accordion`);
// But this expects `acc.vm` to be the parent component
expect(acc.vm.componentRef.state).toEqual(expectedState);

So you should do now

expect(acc.vm.state).toEqual(expectedState);
// or
expect(wrapper.vm.componentRef.state).toEqual(expectedState);

I'm sorry for the regression, but this is how it should be in my opinion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants