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

Add rule to disallow of side-effects inside the render method #90

Closed
stramel opened this issue May 10, 2021 · 3 comments · Fixed by #146
Closed

Add rule to disallow of side-effects inside the render method #90

stramel opened this issue May 10, 2021 · 3 comments · Fixed by #146

Comments

@stramel
Copy link
Contributor

stramel commented May 10, 2021

Potential Rule Name: no-render-side-effects

Disallows the use of "side-effects" inside the render method.

@bennypowers shows a simple case in this tweet: https://twitter.com/PowersBenny/status/1387330749217611776

This could a bit trickier of a rule to implement but would be a good rule to have.

@bennypowers
Copy link

indeed, could be tricky to check for any side effects. could we start with a no-this-assign-in-render or only-local-assign-in-render or something?

@stramel
Copy link
Contributor Author

stramel commented May 10, 2021

indeed, could be tricky to check for any side effects. could we start with a no-this-assign-in-render or only-local-assign-in-render or something?

Yeah, I was thinking along the same lines. Perhaps, also, adding a check for conditional assignments inside the render? I'm unsure what all we would want to consider warnings in a first round pass

@bennypowers
Copy link

given the nature of the problem we might want to take a narrow approach:
local assignments in render are fine
any other assignments, be they to this, window, or higher-scoped variables, are forbidden.

docs link to reasoning, users can turn the rule off if they want

later iterations might add acceptable cases or ignore regexp if there's demand

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 a pull request may close this issue.

2 participants