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

Style guide doc update for React Hooks #2999

Closed
AWolf81 opened this issue May 9, 2019 · 1 comment
Closed

Style guide doc update for React Hooks #2999

AWolf81 opened this issue May 9, 2019 · 1 comment
Assignees
Labels
improvement request 🔨 Issue concerns an existing feature that needs improvement.

Comments

@AWolf81
Copy link
Contributor

AWolf81 commented May 9, 2019

Dev. documentation update

Once PR #2990 is merged (which will be soon) we will have the latest React version - yay 🎉.

This will introduce React hooks where I think we need some guiding notes about how we'd like to use them.

I'll create a PR for the style guide update but I'd like to first discuss here if my usage guide is OK and how others thinks about it.

The below copy will be added to the end of Code style document.

Copy

React Hooks

Existing code will be kept class-based and will only be changed to functional components with hooks if it improves readability or makes things more reusable.

For new components it's OK to use hooks with functional components but don't mix hooks & class-based components within a feature - just for code style / readability reasons.

Read more about hooks in the React hooks introduction.

Discussion

  • Do we need a special heading to the above copy? Right now I would add React Hooks as heading 1.
  • My personal opinion is that I would use hooks if it improves reusability e.g. a useResize would be easier to read / understand than a listener for resize event in the constructor or componentDidMount of a class based component. Both implementations are OK and I'm not seeing it as a must have to use hooks - I would prefer class-based components. What is your opinion? Recommend to continue to use class-based components or always use hooks or use both as I'd recommend?
  • I'm not sure about the point with mixing functional and class-based with-in a feature (2nd last sentence in the copy) - maybe that's also OK to use. What do you think? Maybe I have to create an example to see if it's also OK.
  • Anything else to add to the copy above?
@Rokt33r Rokt33r added the improvement request 🔨 Issue concerns an existing feature that needs improvement. label May 10, 2019
@Rokt33r
Copy link
Member

Rokt33r commented May 10, 2019

@AWolf81
I think it would be easier to discuss if you submit a pr about this. Then, we could easily review and merge. 😄
I'll comment after you create the pr.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement request 🔨 Issue concerns an existing feature that needs improvement.
Projects
None yet
Development

No branches or pull requests

3 participants