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

Fix AnyComponent Type problem #1088

Closed
wants to merge 3 commits into from
Closed

Fix AnyComponent Type problem #1088

wants to merge 3 commits into from

Conversation

zD98
Copy link

@zD98 zD98 commented May 3, 2018

AnyComponent type should be FunctionalComponent or ComponentConstructor.

class Test extends preach.Component<any , any> {
    render(){}
}
// typeof Test is ComponentConstructor

AnyComponent type should be FunctionalComponent or ComponentConstructor.
@coveralls
Copy link

coveralls commented May 3, 2018

Coverage Status

Coverage remained the same at 100.0% when pulling cc0abc0 on zD98:master into 7c3e6fe on developit:master.

@marvinhagemeister
Copy link
Member

marvinhagemeister commented May 3, 2018

We could merge both AnyComponent and ComponentFactory as they are the same with the changes in this PR.

@zD98
Copy link
Author

zD98 commented May 3, 2018

preact-redux connect use preact.AnyComponent to check Component Type.
So should the preact-redux change AnyComponent to ComponentConstructor ?

@scurker
Copy link
Contributor

scurker commented Jul 27, 2018

@zD98 Do you think we could add a test for this? test/ts/hoc-test.tsx currently only tests the definition using ComponentFactory/Component constructor. It should be fairly straightforward to add a test for type checking AnyComponent:

function HOCWrapper<T>(Wrappable: AnyComponent<T>): AnyComponent(T) {
  ...
}

@marvinhagemeister
Copy link
Member

@zD98 I'd love to merge this as this is the right fix. Would you be willing to add a tiny test case for this? Something like @scurker suggested or:

class Foo extends Component {
	render() {
		return <div>Hello</div>
	}
}

function Bar() {
	return <div>World</div>
}

const a: AnyComponent = Foo;
const b: AnyComponent = Bar;

@scurker
Copy link
Contributor

scurker commented Nov 2, 2018

@marvinhagemeister It's been a bit since the original request, so I went ahead and created a duplicate PR #1249 with the necessary test.

@marvinhagemeister
Copy link
Member

Closing this PR as @scurker made a new one with the tests added. Thanks to everyone for working on a fix and getting to the bottom of it :)

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.

None yet

6 participants