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

ReactComponent: remove string refs api and deprecate context #108

Merged
merged 4 commits into from
Nov 6, 2019

Conversation

kLabz
Copy link
Contributor

@kLabz kLabz commented May 19, 2018

The refs API as used with ReactComponentOf<TProps, TState, TRefs> (and other react component typedefs with TRefs) has been deprecated for a while in react.

I am not sure anyone is using it with haxe-react at the moment, but it shouldn't be used any more. However, this may break components using directly ReactComponentOf<TProps, TState, TRefs> instead of ReactComponent and ReactComponentOfX typedefs. The fix is simple but has to be made for the applications to compile.

ReactComponentOfPropsAndState<TProps, TState> has been removed since it has become redundant with ReactComponentOf<TProps, TState>.
However, ReactComponentOfPropsAndState is widely used so it can be enabled for now with -D react_comp_legacy.

Somehow deprecating ReactComponentOfPropsAndState wouldn't work so I had to remove it and add this define.

This PR also deprecates context (see #104).

**/
var refs(default, null):TRefs;
@:deprecated
var context(default, null):Dynamic;
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe instead of @:deprecated we keep them behind -D react_comp_legacy.

Also maybe -D react_deprecated_api is a better name :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure :)

As for the define name, I agree that it sucks. react_deprecated_api is not really future proof, though. Maybe two different defines, like react_deprecated_refs and react_deprecated_context?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I went ahead and did it, feel free to request a define name change if that doesn't suit you.

@kLabz kLabz force-pushed the feature/reactcomponent-updates branch from a34e210 to 8bacd11 Compare May 20, 2018 07:39
@elsassph
Copy link
Contributor

Ok I'm fine with the PR. I wonder if I merge now and we'll get that into a major version update. Though we'd want to have the hxx upgrade as well I suppose before releasing.

@kLabz
Copy link
Contributor Author

kLabz commented May 20, 2018

I guess you can keep this PR around until hxx is ready for a major release.

@kLabz
Copy link
Contributor Author

kLabz commented May 20, 2018

Note: I need to update test/src/react/ReactComponent too.

@kLabz kLabz force-pushed the feature/reactcomponent-updates branch from 051db8d to ea365dd Compare January 7, 2019 06:20
@elsassph elsassph merged commit 5077a25 into massive-oss:master Nov 6, 2019
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