Integrate/typescript: Project shifted to Typescript #251
Conversation
client/src/test/utils/utils.test.js
Outdated
| import { getFullname, makeQueryStringTransformable } from '../../utils/utils'; | ||
|
|
||
| it('Shoud check if getFullname util works', () => { | ||
| expect(getFullname({ firstName: 'Jack', lastName: 'John' })).toEqual( |
There was a problem hiding this comment.
Is in it good to have file that contain data-> user1: {firstName: "abc",lastName:"xyz", fullName: "abc xyz"}?
| 'Jack John' | ||
| ); | ||
| expect(getFullname({ firstName: 'Jack' })).toEqual('Jack'); | ||
| expect(getFullname({ firstName: '', lastName: '' })).toEqual('-'); |
There was a problem hiding this comment.
why do we expect fullname to be '-' if firstname and lastname is empty?
There was a problem hiding this comment.
Because the util method is written this way.
There was a problem hiding this comment.
Don't you think if thats the case we need to fix util method instead of hack test?
There was a problem hiding this comment.
We display a '-' when nothing is provided. If we need to change the util, an issue can be created with the requirement.
| class CommentBox extends Component { | ||
| type Props = SubmitVoid; | ||
|
|
||
| class CommentBox extends Component<Props> { |
There was a problem hiding this comment.
if this component does not have a state we can change it to dump or stateless functional component. If it has state we need to define type for state
| return ( | ||
| <ul className="commentList"> | ||
| {props.comments.map(comment => ( | ||
| <CommentListItem comment={comment} key={comment.id} /> | ||
| <CommentListItem comment={comment} /> |
There was a problem hiding this comment.
does not this produce an error after removing key ?
There was a problem hiding this comment.
This was duplicated. In the container and the component to be rendered. I kept it only in the component. Won't throw a warning.
There was a problem hiding this comment.
Accidentally removed the key. Should have removed from the Item. Done.
| comment: Comment | ||
| }; | ||
|
|
||
| class CommentListItem extends Component<Props> { |
There was a problem hiding this comment.
make it functional component if does not have state
| eventID: string | ||
| }; | ||
|
|
||
| class CommentsBlock extends Component<Props> { |
There was a problem hiding this comment.
make it functional component if does not have state
| description: string, | ||
| }; | ||
|
|
||
| const DescriptionContainer = (props: Props) => { |
There was a problem hiding this comment.
its not logically correct container can not be stateless functional component
There was a problem hiding this comment.
If the container doesn't require any state, why shouldn't it be a functional component?
| @@ -89,7 +94,7 @@ class DropSearch extends Component<Props, State> { | |||
| * - and pass either undefined or extracted object to prop (handleSearchChange method) | |||
| * @returns undefined. | |||
| */ | |||
| handleSearchChange = (e: SyntheticEvent<HTMLButtonElement>) => { | |||
| handleSearchChange = e => { | |||
There was a problem hiding this comment.
can not we add event type here?
There was a problem hiding this comment.
We don't need to add types to everything in our app. Mostly the model should be restricted with types. Including redux logic and component state.
No description provided.