Open
Description
What rule do you want to change?
no-container
Does this change cause the rule to produce more or fewer warnings?
More warnings
How will the change be implemented?
The rule will look for usages of container.innerHTML in addition to looking for usages of methods like container.querySelector()
Example code
const { container } = render(<Greeting />);
expect(container.innerHTML).toContain("Hello");
How does the current rule affect the code?
The current rule allows this code. It only errors if a method like container.querySelector is accessed.
How will the new rule affect the code?
The updated rule would fail on usages of container.innerHTML just as it does with the methods.
Anything else?
Descriptions of the rule would need to be updated from saying "disallow the user of container methods" to "container methods and properties" (or something)
If we found that anyone has use cases where using innerHTML is commonly needed but container methods are not, we could either make it configurable whether it's allowed, or make it a separate rule very similar to no-container
Do you want to submit a pull request to change the rule?
Yes
Metadata
Metadata
Assignees
Labels
Type
Projects
Milestone
Relationships
Development
Participants
Activity
CodingItWrong commentedon Feb 29, 2024
This feels like this'll be as easy to add as the previous rule I added, or easier if it's OK to add this functionality into the existing rule. Happy to do the PR for it 👍
Belco90 commentedon Mar 4, 2024
Hi @CodingItWrong. This is a great suggestion! We would appreciate a PR for implementing this, so definitely go for it by adding the functionality into the existing rule.
CodingItWrong commentedon Mar 6, 2024
Sounds good, @obsoke and I plan to pair on it again over the next few weeks. Thanks for your input!
obsoke commentedon Mar 7, 2024
@Belco90 We were curious how restrictive we want to be when accessing properties on
container. The issue proposes disallowing accessinginnerHTML. Do we want to focus on solely this property for now? What about other properties such asfirstChild,outerHTML,localName, etc.?The rule currently disallows calling any methods on
containerso there is precedent for disallowing properties broadly as well. However, there might be valid use cases for some properties such asfirstChild.Belco90 commentedon Mar 20, 2024
@obsoke I'd say disallowing any method from the
containerproperty would be ideal. I think in general properties likefirstChildshould be avoided.CodingItWrong commentedon Mar 20, 2024
Thanks @Belco90. Just to make sure we follow, we will plan on disallowing any property on the container. This rule already disallows all methods, and since you noted that properties like
firstChildshould be avoided, I follow that your intent is that now all properties should be disallowed in addition to all methods.