First react project, camper leader board. All feedback welcome(Please)

Code Feedback
1

Hey guys,

I’m just done writing my first react app and I would love if you can take a look at the page or code(specially the code) and let me know of any improvements that I could make or things that I could have done better.

code: https://github.com/Randyjp/camper-leader-board
app: http://sleepy-shore-43536.herokuapp.com/

Thanks in advance

2

app.jsx
I’m intrigued by your choice to use global imports. What drives this decision?

CamperAPI.jsx
By convention, jsx extension is reserved for files that have embedded JSX.

CamperAPI.jsx:5
Why not use named export here?

Empty data is still valid data. You should not be throwing an error here, and handle this condition in the view layer: CamperAPI.jsx:15

Board.jsx:9
You don’t have to use constructor just to set the initial state.

export class Board extends React.Component {
  state = {
    camperArray: [],
    isLoading: false
  }
  ...
}

Board.jsx:49
Here you are passing an instance of parent component as a prop to child component.
Does this child component need the whole instance to perform its function?
When component needs to communicate something to parent component, a callback interface will usually suffice.

Sorter.jsx:15
Facebook discourages the use of uncontrolled components.
Make sure this distinction is clear to you. More info here:

Sorter.jsx:24
You can define propTypes as a static prop of class component.

CamperList.jsx:8
Wrapping everything in a div appears to be unnecessary here.
Also what made you chose to extract some logic behind the rendering in its own function, separate from the component? I’m not sure if it’s a bad practice, but it strikes me as unusual.

I haven’t mentioned a lot of other stuff. A lot of good stuff. Your code is good and clean. And you haven’t even used linter! :slight_smile:

1 Like
3

@mpontus thank you so much, this is great feedback and I really appreciate you taking the time to go over my code.

I have to admit there’s not logic behind it. I just did it(that’s bad).

Since it was only one function, looked cleaner.

I was following facebook example: setting state

I was getting undefined when trying to pass just the method, that’s why I ended up using the instance.

I had no idea no of this, makes a lot of sense! thanks!!

Just made my component look nice.

Again, big thanks for your time!! :slight_smile: