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

Tried using react hooks #3

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

Tried using react hooks #3

wants to merge 23 commits into from

Conversation

benishouga
Copy link
Owner

@benishouga benishouga commented Nov 24, 2018

What I did

  • Use react@next and react-dom@next 3285db5 (The latest d.ts corresponds to hooks.)
  • Just replace it, replace it, replace it.
  • Sometimes, commonized API access.

Impressions

pros

  • Simple components have a very good outlook. 9d46d3c
  • It is easier to make common processing depending on the life cycle. 29e9cc0

cons

  • There are cases in which it becomes complicated. 514ce4d
  • The instance variable State was easy to use. When this became a local variable, things to consider increased.

code

before

src/main/client$ sloc .
---------- Result ------------
            Physical :  3685
              Source :  3310
             Comment :  11
 Single-line comment :  10
       Block comment :  1
               Mixed :  10
               Empty :  374
               To Do :  0
Number of files read :  50
------------------------------

after

src/main/client$ sloc .
---------- Result ------------
            Physical :  3294
              Source :  2967
             Comment :  11
 Single-line comment :  10
       Block comment :  1
               Mixed :  11
               Empty :  327
               To Do :  0
Number of files read :  55
------------------------------

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

Successfully merging this pull request may close these issues.

1 participant