Skip to content

Conversation

@Floby
Copy link

@Floby Floby commented Jul 26, 2022

What: This is a bugfix (as far as I can tell).

Why: render() by default appends a new container to a baseElement which is, by default, the global variable document.body. This container ends up containing the resulting markup for the system under test. However, when using the query methods on the RenderResult object, the whole document is scanned for matches, not just the markup resulting from the render() call. This breaks assumptions about the scope in which queries a run : one would expect the getByText and other queries to be scoped to that particular render.

As a matter of fact, this is what happens when the container parameter is given explicitly and baseElement takes it value, however this is not what happens otherwise. The default behaviour (global querying on all renders) is redundant with what screen from @testing-library/dom does, without being very clear about it being global.

How:

  • a test reproducing the issue
  • a lot of code reading to eventually just change a single function call parameter =)

Checklist:

  • Documentation added to the
    docs site (pull request)
  • Tests
  • TypeScript definitions updated
  • Ready to be merged

Floby added a commit to Floby/testing-library-docs that referenced this pull request Jul 26, 2022
@codesandbox-ci
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit dad164e:

Sandbox Source
React Configuration
react-testing-library-examples Configuration

@codecov
Copy link

codecov bot commented Jul 26, 2022

Codecov Report

Merging #1097 (dad164e) into main (c80809a) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##              main     #1097   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            4         4           
  Lines          181       181           
  Branches        34        36    +2     
=========================================
  Hits           181       181           
Flag Coverage Δ
experimental 100.00% <ø> (ø)
latest 100.00% <ø> (ø)
next 100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/pure.js 100.00% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us.

return template.content
}
},
...getQueriesForElement(baseElement, queries),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you check what other testing libraries do? This might've been intended because of portals. But the lack of tests is suspicious.

@nickserv
Copy link
Member

Closing as there hasn't been activity in a year. If this is still needed someone can push a fix for the review comment and ask us to reopen it.

@nickserv nickserv closed this Sep 12, 2023
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.

3 participants