Added fetching data to setup initialOptions. Refactored server. refs #94 #121
Conversation
…host, replaced body parser with express
chosnekk
changed the title
|
I'm sorry, I probably messed up this pull request with commits but I'm totally new to pull requests. |
|
Hi there @adam-p-git! Second, I didn't see any mistake on your commits, they all look great to me. Separated commits for refactor and feature, good message linked to the issue and PR. Awesome.! I'll take a look at the code asap and will give some feedback here. Thanks a lot for your contribution. PS - I invited you to be a team member of the project. Have you received the invite? |
|
Thanks a lot, Tiago! |
tiagostutz
left a comment
tiagostutz
left a comment
There was a problem hiding this comment.
Checking your code I'm thinking about changing one thing on the architecture... instead of having the "model" approach, for example in the app/src/routes/useChoiceBoardModel.ts, we could move the state that is in there ([selectedItems, setSelectedItems]) to the componente itself, ChoiceBoard.ts.
This way, the ChoiceBoard.ts would have the state and would have the reference to useOptions().
What do you think?
|
|
||
| // fetches options and return only choices | ||
| const getOptions = async () => { | ||
| const { data } = await axios({ |
There was a problem hiding this comment.
Why not use the more "native" fetch instead of axios to make the requests? Any thoughts?
I'm asking it because every package we add to the project increases the bundle size.
There was a problem hiding this comment.
Oh, I'm just used to axios, but sure, I can rewrite this with fetch. Keeping bundle size as small as possible makes sense
| var app = express(); | ||
| app.use(bodyParser.json()); | ||
| app.use( | ||
| cors({ |
I'm not sure if I understand this correctly, so please correct me if I'm mistaken. The idea is to move all state into |
|
Exactly that, @adam-p-git ! Perfect. |
|
Oh, great. That was actually what I did in my other project but in javascript. I'll probably have more questions about the app, but I will take a closer look at it tomorrow. |
2 participants
Configured cors, so that app can connect to mock api.
Deleted body-parser due to supporting its features by express from 4.16.
Added new hook useOptions to fetch data from server using react-query.
Added types for options.