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

add TypeScript typings #33

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

add TypeScript typings #33

wants to merge 2 commits into from

Conversation

timse
Copy link

@timse timse commented Jul 26, 2016

adds TypeScript typings

@IRus
Copy link

IRus commented Oct 31, 2016

Any progress on merging this? Looks good for me. cc @gaearon

@IRus
Copy link

IRus commented Nov 22, 2016

Up.

@janslow
Copy link

janslow commented Jan 7, 2017

DocumentTitle isn't the default export (so it should be exported as export = DocumentTitle instead, using a namespace to export the props interface). See the DefinitelyTyped info on this.

> require('react-side-effect')
[Function: withSideEffect]
> require('react-side-effect').default
undefined

@timse
Copy link
Author

timse commented Jan 7, 2017

now :)?

@janslow
Copy link

janslow commented Jan 7, 2017

LGTM

@syadykin
Copy link

rewind() is missing.

@IRus
Copy link

IRus commented May 31, 2017

@syadykin @gaearon is missing

Copy link

@JakeSidSmith JakeSidSmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gaearon could we get this PR merged after these changes? Its' been 2 years...

interface IDocumentTitleProps {
title: string;
}
declare class DocumentTitle extends Component<IDocumentTitleProps, void> {}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

void is not valid in newer React types. You should use {} for back and forward compatibility.

@@ -0,0 +1,6 @@
import {Component} from 'react';
interface IDocumentTitleProps {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rename the interface DocumentTitleProps. It is not recommended to prefix interfaces with I (although its somewhat still up for debate).

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.

5 participants