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

Possible drop of support of using themr as decorator with TypeScript #71

Open
raveclassic opened this issue Jun 28, 2017 · 3 comments
Open

Comments

@raveclassic
Copy link
Contributor

raveclassic commented Jun 28, 2017

Currently I'm trying to rewrite themr in TS to provide consistent up-to-date typings and I'm in the middle of the struggle started in #39
The problem is that switching from correct React.ComponentClass<P> to component constructor type (with new()) is completely broken with strictNullChecks on, here's my comment

@themr('foo') //error - Type 'null' is not assignable to type 'Element'
class FooClass extends React.Component<any, any> {
	render()/*: JSX.Element | null //ok when uncommented*/ {
		return <div>hi</div>;
	}
}

Moreover, decorating a class with @themr complains on every lifecycle method (like componentWillMount) of decorated class as TS can't cast it to ComponentClass. On the other hand, calling themr as a function on existing class works fine - seems like decorators are broken in TS.
Well, at least they are still behind experimental flag.

@mpodlasin @odensc Could you please check if you do use decorators instead of composing HOCs separately from actual component class or sfc which is more natural for react ecosystem in general especially when using recompose?

If it's ok for you and if it's worth dropping experimental decorators, I would open a PR with a fresh new themr fully in typescript.

The good news are that now I'm finally able to overwrite props in decorated component with their non-necessary versions, thanks to this comment
So that themed component does not require theme object to be passed via props but still checks its type if it is present.

UPDATE: still it will be possible and absolutely transparent to use themr as decorator for current ES6 users.

@raveclassic raveclassic changed the title Possible drop of support of using themr as decorator Possible drop of support of using themr as decorator with TypeScript Jun 28, 2017
@odensc
Copy link
Contributor

odensc commented Jun 28, 2017

I don't use themr anymore, but when I did I used decorators.

@Pajn
Copy link

Pajn commented Jul 6, 2017

You'll never be able to create good types for a decorator but you might be able to use what you now have but sill provide decorator support (with bad types) using a function overloads https://www.typescriptlang.org/docs/handbook/functions.html#overloads

If you publish your PR I can take a shot at it.

@raveclassic
Copy link
Contributor Author

I need some time to actualize tests and migrate to latest TS 2.4.1

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

No branches or pull requests

3 participants