Skip to content

Conversation

@0xNeshi
Copy link
Contributor

@0xNeshi 0xNeshi commented Mar 21, 2022

Rename to "set<>" format just to follow the React convention.

@ap-justin
Copy link
Collaborator

@misicnenad, I don't think this is a must follow convention, it's a must though that names suggest that one is state and the other setState. For this case, it's clear that connectOptionsShown is state and showConnectionOptions as something that changes that state

@0xNeshi
Copy link
Contributor Author

0xNeshi commented Mar 22, 2022

@misicnenad, I don't think this is a must follow convention, it's a must though that names suggest that one is state and the other setState. For this case, it's clear that connectOptionsShown is state and showConnectionOptions as something that changes that state

@ap-justin for me it wasn't so obvious that showConnectionOptions was a state setter, looked more like a regular function like any other.
I mean, you are basically right, it is not hard-and-fast rule (at least for now, see end of this comment). But personally, seeing "set" as prefix to a variable "x" helps me to understand this variable is responsible for updating state of "x". Conventions are there to make understanding data types and the purpose of the variables as easy as possible, so why not follow them (unless there's a good reason)?

Additionally, it seems this convention will become more or less enforced in future iterations of eslint-plugin-react, so we might as well update this now.

@ap-justin
Copy link
Collaborator

@misicnenad, I don't think this is a must follow convention, it's a must though that names suggest that one is state and the other setState. For this case, it's clear that connectOptionsShown is state and showConnectionOptions as something that changes that state

@ap-justin for me it wasn't so obvious that showConnectionOptions was a state setter, looked more like a regular function like any other. I mean, you are basically right, it is not hard-and-fast rule (at least for now, see end of this comment). But personally, seeing "set" as prefix to a variable "x" helps me to understand this variable is responsible for updating state of "x". Conventions are there to make understanding data types and the purpose of the variables as easy as possible, so why not follow them (unless there's a good reason)?

Additionally, it seems this convention will become more or less enforced in future iterations of eslint-plugin-react, so we might as well update this now.

@misicnenad, I got this reconsidered and saw that your point is good actually

const [isDropdownOpen, setIsDropdownOpen] = useState(false)

function openDropdown(){
  setIsDropdownOpen(true) 
}

@SovereignAndrey SovereignAndrey merged commit 9021911 into master Mar 23, 2022
@SovereignAndrey SovereignAndrey deleted the rename-state-setter branch March 23, 2022 02:46
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.

4 participants