[Feature]: Removal of `mount()` inside `test.beforeEach()` and `test.afterAll()`

[sand4rt]

🚀 Feature Request

The removal of mount() inside test.beforeEach() and test.afterAll()

Example

This pattern:

const test = baseTest.extend({
  mount({ mount }, use) {
    return use((props) => mount(<Context><Button title="default_title" {...props} /></Context>));
  }
});

test('ok', async ({ mount }) => {
  const component = await mount({ title: 'override_title' });
  await expect(component).toContainText('override_title');
});

Results in a way simpler testbase compared to:

let component;
let props = {};
test.beforeEach(async ({ mount }) => {
  component = await mount(<Context><Button title="default_title" {...props} /></Context>);
  props = {};
});

test('bad', async () => {
  props = { title: 'override_title' };
  await expect(component).toContainText('override_title');
});

Motivation

I've often seen that the use of test.beforeEach and test.afterEach in component testing creates an unmaintainable mess. Kent C. Dodds also discussed this in a blog post and introduced an ESLint rule to address this issue.

One of the great advantages of Playwright fixtures is how they can prevent this issue, so there is no need for an ESLint rule.

[dgozman]

@sand4rt I think ESLint rule and/or docs is the way to fix this, instead of arbitrarily restricting mount() in some environments. There is always a way to introduce unmaintainable mess to the codebase 😄 We should have some "best practices" or "pitfalls" in the documentation describing this. Let me know what you think.

[sand4rt]

I agree that there will always be ways to introduce mess, but i don't think there is any benefit of using test.beforeEach(async ({ mount }) or test.afterEach(async ({ mount }) in component testing though.

That's right. I just don't want us to introduce special cases like this, at least for now, until we see a lot of users bumping into this particular issue.

Some "best practices" or "pitfalls" would definitly be nice anyway. What would be the right place for this?

I guess the same test-components.md? We have not yet figured out a way to structure component testing docs alongside e2e docs, so let's pile up more content in that large guide for now.

[sand4rt]

That's alright. I might add a "best practices" section in test-components.md later on.

I don't think there are many people who will report this, but let's see. Any chance the feature-components tag could be added? We can also see whether the counter is increasing here: https://github.com/search?q=beforeEach%28async+%28%7B+mount+%7D%29&type=code (43 matches ATM).

[dgozman]

We can also see whether the counter is increasing here: https://github.com/search?q=beforeEach%28async+%28%7B+mount+%7D%29&type=code (43 matches ATM).

Thank you for the link! Looking at the usages, some of them look totally fine to me. Maybe it's not that obvious a decision, and folks should decide for themselves 😄

[sand4rt]

Thank you for the link! Looking at the usages, some of them look totally fine to me. Maybe it's not that obvious a decision, and folks should decide for themselves 😄

Some of them look fine now, but will look terrible when they evolve, especially if different props are needed for a few tests. In my opinion, it's better to provide users the right handles rather than trying to support everything in component testing. This same mistake was made during the Enzyme days and was a painfull lesson learned. Might be interesting to save a few links and set a reminder to check in after a few months :D

I totally respect that you don't want to add an special case just for component testing tho!