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

Inconsistent .text() behaviour on components using v-if #342

Closed
twoBoots opened this issue Feb 2, 2021 · 6 comments · Fixed by #351
Closed

Inconsistent .text() behaviour on components using v-if #342

twoBoots opened this issue Feb 2, 2021 · 6 comments · Fixed by #351
Labels
bug Something isn't working

Comments

@twoBoots
Copy link

twoBoots commented Feb 2, 2021

Inconsistent .text() behaviour on components using v-if

❌ When mounting a component that has a single root node with a v-if, when the v-if is falsey wrapper.text() returns the text of the HTML comment <!--v-if--> inserted in the DOM. See RootIf.vue below.

✅ When mounting a component that has a single node with a v-if wrapped with a <div>, when the v-if is falsey wrapper.text() returns and empty string. See WrappedIf.vue below.

✅ When mounting a component that has two root nodes, first with v-if and second with v-else, wrapper.text() returns only the text of the visible node. See RootIfElse.vue below.

Steps to reproduce

"vue": "3.0.0",
"@vue/test-utils": "2.0.0-beta.5",

RootIf.vue

<template>
  <div v-if="error">
    <span> error text </span>
  </div>
</template>

<script>
  export default {
    name: 'rootif',
    data: () => ({ error: false }),
  };
</script>

RootIfElse.vue

<template>
  <div v-if="error">
    <span> error text </span>
  </div>
  <div v-else> else text </div>
</template>

<script>
  export default {
    name: 'rootifelse',
    data: () => ({ error: false }),
  };
</script>

WrappedIf.vue

<template>
  <div class='im-a-wrapper'>
    <div v-if="error">
      <span> error text </span>
    </div>
  </div>
</template>

<script>
  export default {
    name: 'wrappedif',
    data: () => ({ error: false }),
  };
</script>

__tests__/wat.spec.js

import { mount } from '@vue/test-utils';
import RootIf from '../RootIf';
import RootIfElse from '../RootIfElse';
import WrappedIf from '../WrappedIf';

describe('wrapper.text() behaviour', () => {
  let wrapper;

  describe('RootIf', () => {
    test('renders empty', async () => {
      wrapper = mount(RootIf);

      expect(wrapper.text()).toBe('');
    });
  });

  describe('RootIfElse', () => {
    test('renders else text', async () => {
      wrapper = mount(RootIfElse);

      expect(wrapper.text()).toBe('else text');
    });
  });

  describe('WrappedIf', () => {
    test('renders empty', async () => {
      wrapper = mount(WrappedIf);

      expect(wrapper.text()).toBe('');
    });
  });
});

Expected behaviour

When mounting a component that has a single root node with a v-if, when the v-if is falsey wrapper.text() should return an empty string.

Actual behaviour

 FAIL  src/app/__tests__/wat.spec.js
  wrapper.text() behaviour
    RootIf
      ✕ renders empty (2ms)
    RootIfElse
      ✓ renders else text (1ms)
    WrappedIf
      ✓ renders empty (1ms)

  ● wrapper.text() behaviour › RootIf › renders empty

    expect(received).toBe(expected) // Object.is equality

    Expected: ""
    Received: "v-if"

      11 |       wrapper = mount(RootIf);
      12 | 
    > 13 |       expect(wrapper.text()).toBe('');
         |                              ^
      14 |     });
      15 |   });
      16 | 

      at Object.<anonymous> (src/app/__tests__/wat.spec.js:13:30)

Test Suites: 1 failed, 1 total
Tests:       1 failed, 2 passed, 3 total
Snapshots:   0 total
Time:        0.127s, estimated 1s

wrapper.text() returns "v-if"

Possible Solution

Possible workaround is to add unnecessary additional empty root nodes with v-else to get expected behaviour while running tests.

@twoBoots
Copy link
Author

twoBoots commented Feb 2, 2021

Apologies if this issue should have been submitted against https://github.com/vuejs/vue-test-utils-next 🤦

@afontcu afontcu transferred this issue from vuejs/vue-test-utils Feb 2, 2021
@afontcu
Copy link
Member

afontcu commented Feb 2, 2021

Apologies if this issue should have been submitted against https://github.com/vuejs/vue-test-utils-next 🤦

No worries, just transferred the issue to the right repo :)

@lmiller1990
Copy link
Member

Hmmm interesting one. I don't have a solution off the top of my head, I guess we just write some code (regexp?) to remove the comments when using text?

That should be quite easy. Would you like to make a PR?

If someone has a better solution, by all means share!

@lmiller1990 lmiller1990 added the bug Something isn't working label Feb 3, 2021
@twoBoots
Copy link
Author

twoBoots commented Feb 3, 2021

I'm not sure if a regexp would solve this as comment content is not returned by .text() (using textContent under the hood) in any other scenario.

e.g.

RootIfWithComment.vue

<template>
  <div v-if="error">
    <span> error text </span>
  </div>
  <!--a comment-->
</template>

<script>
  export default {
    name: 'rootifwithcomment',
    data: () => ({ error: false }),
  };
</script>

Mounting this component, wrapper.text()returns an empty string in this case.

The issue only occurs on components with single root nodes using v-if. Here wrapper.text() returns "v-if" and not the full comment <!--v-if--> so a regex to strip HTML comments wouldn't catch this one.

@cexbrayat
Copy link
Member

I understand this is surprising, but this works as expected right?

If you have a comment node, textContent returns the comment. So if your root element is a v-if with a false condition, than the generated DOM only contains a comment, hence v-if returned for wrapper.text(). If you add a wrapper element than your test is probably OK. Or switch your assertion to check if the element is in the DOM rather than checking its textContent.

We could also consider returning an empty string for comment nodes as I suspect nobody really needs to get the content of a comment (it is a breaking change though).

@cexbrayat
Copy link
Member

I opened #351 if we want to go ahead and return an empty string for comments

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

Successfully merging a pull request may close this issue.

4 participants