Skip to content

Conversation

MSroczynski3
Copy link
Contributor

@MSroczynski3 MSroczynski3 commented Jul 28, 2025

In this PR, I extended the src/rules/prefer-web-first-assertions.ts rule to detect another two flaky methods:

-   allInnerTexts()
-   allTextContents().

Additionally, I added handling for options argument that is useful, for example, when replacing innerText() with toHaveText() with useInnerText option. Tests for all changes were added.

This is a follow-up to #362 .

@mskelton
Copy link
Member

This isn't the right approach IMO. This isn't a one-to-one replacement, since the behavior of allTextContents is different from toHaveText.

I think we should add a lint error, but not autofix.

@MSroczynski3
Copy link
Contributor Author

@mskelton I see your point here. My thought process was connected with the fact that toHaveText() is designed to handle multiple elements and multiple text values well. That being said this rule would get lost with this problem:

        const myText = page.locator('foo li').allTextContents();
        expect(myText).toEqual(['foo', 'bar', 'baz'])`

I tried a simplified approach. Could you please let me know your thoughts?

@MSroczynski3 MSroczynski3 force-pushed the rule/prefer-to-have-text branch from 0b1c34f to 1cd4c02 Compare July 30, 2025 18:59
@MSroczynski3 MSroczynski3 changed the title Extend prefer-web-first-assertion #362 - Extend prefer-web-first-assertion Jul 30, 2025
@mskelton
Copy link
Member

We can apply an autofix if the autofix is a 1x1 match of behavior, so the example you gave would work.

@MSroczynski3
Copy link
Contributor Author

I can give it a try, but taking into account how much logic there is in the rule already, I am not sure if it won't complicate things too much. Would this merit a separate rule? Probably unlikely...

@MSroczynski3
Copy link
Contributor Author

MSroczynski3 commented Aug 12, 2025

@mskelton I spent some time thinking about this. I think that this rule is in a good place. Adjusting it further will bloat the function for relatively little added value. Do you think we can proceed and merge this PR?

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.

2 participants