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

Use {} instead of void for component state to avoid build errors. #21

Merged
merged 2 commits into from
Aug 9, 2017

Conversation

rozzzly
Copy link
Contributor

@rozzzly rozzzly commented Aug 6, 2017

Updates to TypeScript/the React type definitions have caused issues for me when a component uses void instead of {} to indicate that a component doesn't make use of state. With default generics in typescript@2.3 and the subsequent update over at DefinitelyTyped you can just omit the void so your class declaration looks like:

class MyComponent extends React<MyComponentProps> {
}

And things will work just fine. However so the type definitions in this repo are more backwards compatible I opte to substitute {} in for void so pre 2.3 consumers don't have any issues.

Here's the build error I get when a component uses void:

[at-loader] Checking finished with 2 errors
[at-loader] ./src/common/components/Modal/index.tsx:107:13
    TS2605: JSX element type 'Fill' is not a constructor function for JSX elements.
  Types of property 'setState' are incompatible.
    Type '{ <K extends never>(f: (prevState: void, props: Props) => Pick<void, K>, callback?: () => any): v...' is not assignable to type '{ <K extends never>(f: (prevState: {}, props: any) => Pic
k<{}, K>, callback?: () => any): void; <...'.
      Types of parameters 'f' and 'f' are incompatible.
        Types of parameters 'prevState' and 'prevState' are incompatible.
          Type 'void' is not assignable to type '{}'.

[at-loader] ./src/common/components/Modal/index.tsx:139:13
    TS2605: JSX element type 'Fill' is not a constructor function for JSX elements.

@rozzzly
Copy link
Contributor Author

rozzzly commented Aug 7, 2017

If someone wanted to take a look at the failing tests, I'd appreciate it. It looks like they're related to the type definitions for the project's dependencies and not due to the change I made.

@camwest
Copy link
Owner

camwest commented Aug 8, 2017

@rozzzly can you try again with the latest master?

@rozzzly
Copy link
Contributor Author

rozzzly commented Aug 9, 2017

@camwest looks like everything is good to go

@camwest camwest merged commit 7905a90 into camwest:master Aug 9, 2017
@rozzzly
Copy link
Contributor Author

rozzzly commented Aug 11, 2017

@camwest could you publish a new version to NPM? thanks

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.

2 participants