#116 07 form update dependencies #122

carlostxm · 2017-11-05T20:48:46Z

Updating dependencies.

It was needed to refactor the App due to the changes in react-router.

component moved to react-router-dom
Nesting routes are not allowed in react-router v4 so router.tsx was refactored using
hashHistory is removed in v4, so Router was changed by which contains history.

For more details check @arp82 comment in #116.

…t-samples

…ory has been disabled to test changes in isolation. It will be fixed in the next commit.

…ger managed separately but <HashRouter> creates the history for you so it's accesible from children <Route>s

This reverts commit 965ef70.

…ck()

This reverts commit 0b0a697.

arp82 · 2017-11-10T12:09:30Z

07 Form/src/components/member/pageContainer.tsx

-  constructor() {
-    super();
+interface Props {
+  history: PropTypes.object.isRequired;


This is OK, it is using the default React typing from prop-types library. I wonder though if there would be a better way to do this. @brauliodiez, any suggestions?

For type history you have to:

import { History } from 'history'; ... interface Props { history: History; }

Since is typescript we should not use prop-types, just interface

arp82

Good job!

nasdan · 2017-11-15T10:06:07Z

07 Form/package.json

-    "react-router": "^3.0.5",
+    "react": "^16.0.0",
+    "react-dom": "^16.0.0",
+    "react-router": "^4.2.0",


react-router is not necessary because it's the core. I mean, react-router is used in react-router-dom

nasdan · 2017-11-15T10:06:24Z

07 Form/package.json

-    "@types/react-router": "^3.0.11",
+    "@types/react": "^16.0.20",
+    "@types/react-dom": "^16.0.2",
+    "@types/react-router": "^4.0.16",


nasdan · 2017-11-15T10:07:37Z

07 Form/readme.md

@@ -109,21 +109,25 @@ export * from './members';
 ### ./src/router.tsx
 ```diff
 import * as React from 'react';
-import { Router, Route, IndexRoute, hashHistory } from 'react-router';
+import { Route, Switch } from 'react-router';


Use imports from react-router-dom

nasdan · 2017-11-15T10:21:46Z

07 Form/src/components/member/pageContainer.tsx

@@ -50,8 +54,8 @@ export class MemberPageContainer extends React.Component<{}, State> {
      <MemberPage
        member={this.state.member}
        onChange={this.onFieldValueChange}
-        onSave={this.onSave}
+        onSave={this.onSave(this.props)}


not necessary passing props as argument because the onSave method is define inside class.

You could:

private onSave = () => { .... this.props.history.goback(); .... }

nasdan · 2017-11-15T10:22:16Z

07 Form/src/router.tsx

@@ -1,17 +1,21 @@
 import * as React from 'react';
-import { Router, Route, IndexRoute, hashHistory } from 'react-router';
+import { Route, Switch } from 'react-router';


Use imports from react-router-dom

nasdan · 2017-11-15T11:26:17Z

There is a typings error in ./src/components/members/page.tsx pending to pass:

...
-  constructor() {
+  constructor(props) {
-    super();
+    super(props);
}
...

carlostxm · 2017-11-16T19:09:36Z

@nasdan, thank your for your comments!

arp82

Seems like you covered everything from Nasdan's review. Good job!

arp82

I think you missed the reference to react-router in line 396 of readme.md. Should it not be react-router-dom instead?

carlostxm · 2017-11-19T09:18:09Z

You're right @arp82, thanks. I've just updated the readme. This is the last issue opened, if there is not more issues to fix in this PR, please @nasdan @brauliodiez merge and close this inifinite issue :)

Carlos and others added 15 commits November 1, 2017 00:05

updating dependencies

965ef70

Merge branch 'master' of https://github.com/Lemoncode/react-typescrip…

a7c5683

…t-samples

updating all dependencies except react-router

f815e9a

Upgrading react-router to v4. Fixing Link and Router components. Hist…

1fad5a1

…ory has been disabled to test changes in isolation. It will be fixed in the next commit.

Fixing problem with hashHistory. In react-router v4 history is no lon…

c08728f

…ger managed separately but <HashRouter> creates the history for you so it's accesible from children <Route>s

Updating readme

2c0577c

Revert "updating dependencies"

fabe031

This reverts commit 965ef70.

Merge remote-tracking branch 'Lemoncode/master'

daa5723

Merge branch '07-form-update-dependencies'

3830e00

updating with remote origin changes

716fc85

adding package-lock.json to gitignore

0b0a697

Using the right history. Now we can use goBack() method instead of ba…

a5c9fb2

…ck()

'this' is not necessary

02911c9

Updating readme with last changes

1581653

Revert "adding package-lock.json to gitignore"

0b2ebf7

This reverts commit 0b0a697.

brauliodiez requested a review from arp82 November 9, 2017 17:36

arp82 reviewed Nov 10, 2017

View reviewed changes

arp82 approved these changes Nov 10, 2017

View reviewed changes

carlostxm closed this Nov 13, 2017

carlostxm reopened this Nov 13, 2017

nasdan reviewed Nov 15, 2017

View reviewed changes

carlostxm added 4 commits November 16, 2017 19:45

Typing using History instead of Proptypes

5ae6051

Removing react-router dependency since is used in react-router-dom

c6129db

using imports from react-router-dom

09c91e9

Fixing props

94edbd4

carlostxm added 2 commits November 16, 2017 19:59

Bind is not needed since onSave is declared using arrow function

ddf95dc

Updating readme

036b9e2

arp82 approved these changes Nov 16, 2017

View reviewed changes

arp82 suggested changes Nov 16, 2017

View reviewed changes

Updating readme with last changes

d733635

arp82 approved these changes Nov 20, 2017

View reviewed changes

brauliodiez merged commit c116d96 into Lemoncode:master May 7, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#116 07 form update dependencies #122

#116 07 form update dependencies #122

carlostxm commented Nov 5, 2017

arp82 Nov 10, 2017

nasdan Nov 15, 2017

brauliodiez Nov 15, 2017 •

edited

Loading

arp82 left a comment

nasdan Nov 15, 2017

nasdan Nov 15, 2017

nasdan Nov 15, 2017 •

edited

Loading

nasdan Nov 15, 2017

nasdan Nov 15, 2017

nasdan commented Nov 15, 2017

carlostxm commented Nov 16, 2017

arp82 left a comment

arp82 left a comment

carlostxm commented Nov 19, 2017

#116 07 form update dependencies #122

#116 07 form update dependencies #122

Conversation

carlostxm commented Nov 5, 2017

arp82 Nov 10, 2017

Choose a reason for hiding this comment

nasdan Nov 15, 2017

Choose a reason for hiding this comment

brauliodiez Nov 15, 2017 • edited Loading

Choose a reason for hiding this comment

arp82 left a comment

Choose a reason for hiding this comment

nasdan Nov 15, 2017

Choose a reason for hiding this comment

nasdan Nov 15, 2017

Choose a reason for hiding this comment

nasdan Nov 15, 2017 • edited Loading

Choose a reason for hiding this comment

nasdan Nov 15, 2017

Choose a reason for hiding this comment

nasdan Nov 15, 2017

Choose a reason for hiding this comment

nasdan commented Nov 15, 2017

carlostxm commented Nov 16, 2017

arp82 left a comment

Choose a reason for hiding this comment

arp82 left a comment

Choose a reason for hiding this comment

carlostxm commented Nov 19, 2017

brauliodiez Nov 15, 2017 •

edited

Loading

nasdan Nov 15, 2017 •

edited

Loading