Skip to content
This repository was archived by the owner on Mar 4, 2020. It is now read-only.

Access modifiers and order of members in React components #936

Closed
bmdalex opened this issue Feb 20, 2019 · 3 comments
Closed

Access modifiers and order of members in React components #936

bmdalex opened this issue Feb 20, 2019 · 3 comments
Labels
vsts Paired with ticket in vsts

Comments

@bmdalex
Copy link
Collaborator

bmdalex commented Feb 20, 2019

Access modifiers and order of members in React components

The goal of this issue is to come up with certain guidelines and best practices for writing React components when it comes to:

1. Leveraging TypeScript functionality of access modifiers (protected, private) and using them for React class components

Arguments for:

  • we're using TypeScript as main language for this project so we strive to leverage functionality (access modifiers in this case);
  • showcase the public API;
  • helpful for avoiding dead code (it's easy to forget about an unused method that is public by default and it won't be detected by the linter if we don't use the private modifier);

Arguments against:

  • closer to ES6 and React standards with ES6
  • easier to adopt by JS devs to it includes a larger community
  • possibility that with hooks we won't write as many class components

We need YES / NO decision and maybe enforce a eslint rule?

2. Enforce a convention for ordering methods within a component

a. React convention:

  • described here:

    1. static methods and properties
    2. lifecycle methods
    3. custom methods
    4. render method
  • looks like the generic convention adopted by the React community

b. TypeScript / OOP convention:

  • simplified version (static members come before instance members):

    1. private members
    2. public members
    3. constructor
    4. public methods
    5. private methods
  • we're writing TypeScript code so we should take this into account when making a decision;

  • is having events handlers before render method a good way to structure it necessarily?
    One might argue that when you read the code you want to start with the entry points (the public methods, render in this case) and continue to scroll down as if it was a story, without too many jumps to the private methods that are called from the public ones (and constructor, that would also be placed at the beginning of the class).
    (full comment here)

We need a. / b. decision and maybe enforce a eslint rule or install this plugin?

@layershifter
Copy link
Member

Convention is actually exists, we should just follow it: https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/sort-comp.md

My vote is to follow patterns that exists in React world to make the Stardust more friendly for community users.

@bmdalex bmdalex changed the title Access modifiers and order of members in React components Access modifiers and order of members in React components and rest of code Feb 20, 2019
@bmdalex bmdalex changed the title Access modifiers and order of members in React components and rest of code Access modifiers and order of members in React components Feb 20, 2019
@pkumarie2011 pkumarie2011 added the vsts Paired with ticket in vsts label Feb 20, 2019
@levithomason
Copy link
Member

levithomason commented Mar 6, 2019

Let's not spend cycles on this one. Classifying as bikeshedding. We have many important functional milestones to hit. Let's get those in order and allow automation / fix to handle formatting things.

  • We'll use remove TSLint for ESLint: chore(package): add ESLint #600
  • Render method last (community standard)
  • Remove access modifiers (don't make sense in React)

Also, reiterating, let's not discuss this one anymore, but let's automate it, onward! :)

@jurokapsiar
Copy link
Contributor

closing as there was no activity

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
vsts Paired with ticket in vsts
Projects
None yet
Development

No branches or pull requests

5 participants