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: Component is not re-rendered after changing props when a recursive component is rendered #2039

Closed
Sumolari opened this issue Apr 29, 2023 · 6 comments · Fixed by #2057
Closed
Labels
bug Something isn't working

Comments

@Sumolari
Copy link

Describe the bug

In render trees where there is a recursive component being rendered, changing the props of the root component does not re-render the component tree. This was introduces in version 2.3.1.

To Reproduce
https://stackblitz.com/edit/vitest-dev-vitest-mbp4xz?file=test/basic.test.ts

Expected behavior
When the props of the root component change, the children that receive those props should be updated, too, regardless of any other component being recursive or not.

Related information:
The issue can be reproduced in @vue/test-utils v2.3.1, v2.3.2 and v2.4.0-alpha.0. The issue does not affect v2.3.0.

  System:
    OS: Linux 5.0 undefined
    CPU: (4) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 0 Bytes / 0 Bytes
    Shell: 1.0 - /bin/jsh
  npmPackages:
    @vue/test-utils: 2.3.1 => 2.3.1 
    vitest: latest => 0.30.1 
    vue: latest => 3.2.47 

Additional context
Note that the issue is about non-recursive components not being updated when part of a tree that also includes other recursive-components, even if the non-recursive components are direct children of the root component.

In the test scenario I'm trying to render a simple folder structure like this one:

- Root
    - The Folder
    - The File

Where both Root and The Folder and FolderItem components while The File is a FileItem component. The test first renders the tree without passing an icon prop and then changes the icon prop to a different value. The icon prop is passed directly to both FolderItem and FileItem components.

I was expecting FileItem to be updated and finding the new value of the prop when calling its .props('item'). This is what happens in @vue/test-utils v2.3.0 and also what happens in all versions when I don't try to render any other FolderItem component in that tree.

However, when I render another FolderItem component, in @vue/test-utils v2.3.1 and later the FileItem component never gets updated and the test always get the original value for the item prop.

I believe this is a regression: even if all FolderItem instances but the root one are stubbed, the FileItem component is a direct child of the root FolderItem so I don't think it should be affected by its sibling being stubs or not. Please, correct me if I misunderstood something and this is actually the intended behavior.

@Sumolari Sumolari added the bug Something isn't working label Apr 29, 2023
@lmiller1990
Copy link
Member

lmiller1990 commented May 1, 2023

I got is to passing using mount:

import { mount } from '@vue/test-utils';
import FolderItem from '../components/FolderItem.vue';
import FileItem from '../components/FileItem.vue';


// The test below fails in v2.3.1 but works fine in v2.3.0
test('Updates prop passed from parent to non-recursive children even when there is a recursive component rendered', async () => {
  const wrapper = mount(FolderItem, {
    props: {
      name: 'Root',
      items: [
        {
          name: 'The Folder',
          items: [],
        },
        { name: 'The file' },
      ],
    },
  });

  expect(wrapper.findComponent(FileItem).props('icon')).toBeUndefined();

  await wrapper.setProps({
    icon: 'fake-icon',
  });

  expect(wrapper.findComponent(FileItem).props('icon')).toBe(
    'fake-icon'
  );
});

I think #1979 broke something in shallowMount. We may need to revert this, although it does fix another bug.

In general, I'd really recommend mount - it does not suffer from any of the inconsistencies/non-production like bugs that shallowMount does. Is mount an option for you?

@Sumolari
Copy link
Author

Sumolari commented May 1, 2023

Thanks for checking this out @lmiller1990!

I'm glad there's a workaround for this, unfortunately I can't use it in my the real application: there are some inner components I must stub and as soon as I stub some of them using stubs option of mount, I end up in the same situation

Hopefully this workaround will help other folks running into this issue until it gets fixed

@lmiller1990
Copy link
Member

Can you describe what type of components you need to stub? Maybe we can build out something better than simply stubbing components to facilitate testing, or maybe there is an alternative (most test runners have a stub or mock feature, maybe that would be an option).

@Sumolari
Copy link
Author

Sumolari commented May 7, 2023

Using mount is a bit cumbersome: I have to mock useI18n from vue-i18n because some child component deep in the tree renders a localized string, also some components use Pinia, which I then must mock or provide a test store

I'm use vitest as test running and I can mock all of that but it's just too much effort and complicates the test setup too much when the component I'm trying to test doesn't use any of them directly

@lmiller1990
Copy link
Member

I see. I'd generally just test my components with a real store and i18n - more coverage, and it's more real.

If you want to try and the commit that introduced this issue, that might be a good place to start debugging.

@freakzlike
Copy link
Collaborator

v2.3.1 introtuced the stub of a recursive component. Therefore using v2.3.0 would have the same effect as not stubbing the recusrive component at all. I'm still investigating

lmiller1990 pushed a commit that referenced this issue May 21, 2023
…2057)

* Reproduction for #2039

* fix(stub): re-render of recursive component using wrong cached stub
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.

3 participants