refactor: add typescript [ECO-960] #33
Conversation
surajcontentstack
left a comment
surajcontentstack
left a comment
There was a problem hiding this comment.
Couple of changes:
1- I think its better not to use Object as type. It will create conflict with the native Object.
Rename it to something more relevant
2- Moving all the type definitions out into separate file is not required. Doing so is good practice for commonly used types. But for components prop types, it makes sense to define them in the component file itself. That way its easy to quickly check the props.
typescript/layout.ts
Outdated
| import { Image } from "./action"; | ||
| import { Component } from "../typescript/component"; | ||
|
|
||
| type Object = { |
There was a problem hiding this comment.
I think its better not to use Object as type. It will create conflict with the native Object.
Rename it to something more relavant
typescript/pages.ts
Outdated
| import { Image } from "../typescript/action"; | ||
| import { Entry, HeaderProps ,FooterProps } from "./layout"; | ||
|
|
||
| type Object = { |
There was a problem hiding this comment.
I think its better not to use Object as type. It will create conflict with the native Object.
Rename it to something more relavant
abhishek305
left a comment
abhishek305
left a comment
There was a problem hiding this comment.
@karantalapalli
make sure to not use any type only use it if the object type is unknown
components/footer.tsx
Outdated
| function buildNavigation(ent, ft) { | ||
| const [getFooter, setFooter] = useState(footer); | ||
|
|
||
| function buildNavigation(ent: Entry, ft: any) { |
There was a problem hiding this comment.
footer response should not be any & can be provided as an object(refer to type created for footer)
components/header.tsx
Outdated
| const [getHeader, setHeader] = useState(header); | ||
|
|
||
| function buildNavigation(ent, hd) { | ||
| function buildNavigation(ent: Entry, hd: any) { |
There was a problem hiding this comment.
header response should not be any & can be provided as an object(refer to type created for header)
components/layout.tsx
Outdated
| blogList && (jsonObj.blog_post = blogList); | ||
|
|
||
| function buildNavigation(ent, hd, ft) { | ||
| function buildNavigation(ent: Entry, hd: any, ft: any) { |
There was a problem hiding this comment.
similar to what mentioned in above comments for header/footer
components/blog-list.tsx
Outdated
|
|
||
| function BlogList({ bloglist }) { | ||
| function BlogList({ bloglist }: { bloglist: BloglistProps }) { | ||
| let body = typeof bloglist.body === 'string' && bloglist.body.substr(0, 300); |
There was a problem hiding this comment.
make let body:string once
components/blog-list.tsx
Outdated
| let body = typeof bloglist.body === 'string' && bloglist.body.substr(0, 300); | ||
| const stringLength = body.lastIndexOf(' '); | ||
| body = `${body.substr(0, Math.min(body.length, stringLength))}...`; | ||
| const stringLength = (body as string).lastIndexOf(' '); |
There was a problem hiding this comment.
remove body as string from here
package.json
Outdated
| "name": "contentstack-nextjs-starter-app", | ||
| "description": "A starter app for Contentstack and Nextjs", | ||
| "version": "1.4.0", | ||
| "version": "0.1.0", |
There was a problem hiding this comment.
version should not down grade from 1.4.0 to 0.1.0. this should be 2.0.0
refactor: add typescript [ECO-960]