Skip to content
This repository has been archived by the owner on Jul 21, 2021. It is now read-only.

Performant React Forms #1

Open
wants to merge 60 commits into
base: master
Choose a base branch
from
Open

Performant React Forms #1

wants to merge 60 commits into from

Conversation

nold2
Copy link

@nold2 nold2 commented Dec 21, 2018

  1. Implemented Lazy load for the custom select fields
  2. Implemented React.memo for the custom select fields
  3. Added the appropriate proptypes and default props for the custom select fields
  4. Implemented React Suspense to lazy load the custom fields
  5. implemented componentDidMount LifeCycle to ensure a single trip to server to retrieve the Province data
  6. Implemented ComponentDidUpdate LifeCycle to ensure that once a province change, cities, districts and subdistricts will change as well same goes for other fields

@nold2
Copy link
Author

nold2 commented Dec 27, 2018

For me, why did you use await at all?

So personally I like async await syntax better. Since it is shorter and less chaining

But in this case I want to map the api result into the appropriate object shape that the react-select library accepts, hence the then keyword.

Since it is still an un-resolved promise I use the await keyword.

Is there a better pattern that you would like to see? Like another chain of .then(onSuccess, onFail) where onSuccess will do the setState?

@nold2
Copy link
Author

nold2 commented Dec 27, 2018

Well React.memo prevents any unwanted re-renders by doing a shallow comparision of the props.

I was testing my code with React dev tools and noticed whenever I select an option other CustomSelect Components keeps on re-rendering.

With React.memo, I was able to only re-render the selected component itself to display the value, and the CustomSelect below it to get the options.

Not the whole CustomSelect component attached to the container

@nold2
Copy link
Author

nold2 commented Dec 27, 2018

Sure, having exact prop would be nice. i will add it

@nold2
Copy link
Author

nold2 commented Dec 27, 2018

Hi guys in this latest push, I managed to implement Redux(actions, actionTypes and reducer) and ReduxForm(form submission where output is displayed by console.log and form validation ).

So no more promises with an await keyword

It's getting late currently I really want to implement some features:

  1. reseting the cities, districts and subdistricts if user change the province, (same goes for changing the city, the district and subdistricts should change)
  2. I haven't really get the chance to implement intelligently predict user's location
  3. UI library or any fancy css modules or css in JS

But so far this MR is working according to the requirement regarding efficient API fetch and redux,
P.S. for now if you visit https://nold2.github.io/frontend-test/#/. Open the chrome dev tools you will see the redux actions getting dispatched.

@vferdiansyah @onggunhao @kaiyuanneo @faisaldanacita Would this be enough for now? I don't mind adding/removing stuffs that are important

Thank you

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

Successfully merging this pull request may close these issues.

3 participants