-
Notifications
You must be signed in to change notification settings - Fork 29.9k
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
Defintitions for react-router v1.0.0 #6073
Conversation
I'm getting an error with typescript 1.6.2, both with VSCode intellisense and building with browserify + tsify.
I have Any ideas? |
I realized this is based on a newer react.d.ts than what is published. Copied in the latest changes myself. |
any update on when this will be merged? |
i use this definitions. good work |
Can we get this merged?@vvakame |
I just realised that this is also adding another library's type definition (npm history package). There's currently a definition under history but it corresponds to the history.js npm package. Should we rename the existing one to history.js and add history as it's own type definition? |
It would be theoretically right but in practice will break existing dependencies for number of projects. |
interface RouteProp { | ||
name?: string; | ||
path?: string; | ||
handler?: React.ComponentClass<any>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this not be component?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This what? NotFoundRouteClass?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The route property handler is now named component.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes you are right. Sorry this is the wrong file.
umm... we can't get review by authors. @sergey-buturlakin please split https://www.npmjs.com/package/history package definition to other pull request first. |
Hi guys, sorry for the delay, didn't have time to review. EDIT: Maybe test and eventually adapt with stable react-router v1.0.0? |
@sergey-buturlakin In new React 0.14 definitions there is no longer definition for HTMLAttributesBase which you use in your react-router definitions |
Just a note: ReactRouter 1.0.0 (stable) was released Monday, Nov 9. Would be great to test it against that release and mark it as 1.0.0 instead of rc1. I'm going to be testing it soon, but not sure how soon or how fully. |
} | ||
interface Redirect extends React.ReactElement<RedirectProps> {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should extend React.ComponentClass
, rather than React.ReactElement
I will update the PR to the latest History and Router versions this week |
getQuery(): {}; | ||
isActive(to: string, params?: {}, query?: {}): boolean; | ||
interface HistoryRoutes { | ||
isActive(pathname: H.Pathname, query: H.Query): boolean |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
query
should be optional here
Fix #5939
Also definitions for https://www.npmjs.com/package/history provided