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 getRangeBehavior to allow non-string calls #1216

Closed
wants to merge 1 commit into from

Conversation

xuorig
Copy link
Contributor

@xuorig xuorig commented Jun 15, 2016

Fixes #1208

That invariant doesn't seem to make sense in that function, removing it will allow us to use rangeBehavior functions with other types than string.

@josephsavona

@ghost ghost added the CLA Signed label Jun 15, 2016
@kassens
Copy link
Member

kassens commented Jun 15, 2016

Thanks for the fix! I checked the internal discussion and it indeed seems like an oversight.

Would you mind adding a test case to getRangeBehavior?

@kassens kassens self-assigned this Jun 15, 2016
@xuorig xuorig force-pushed the fix-range-beha-fn branch from 25ce3a1 to d74cb06 Compare June 15, 2016 17:14
@xuorig
Copy link
Contributor Author

xuorig commented Jun 15, 2016

Added a regression test @kassens !

behavior
);
behaviors[call.name] = behavior;
behaviors[call.name] = call.value;
Copy link
Contributor

Choose a reason for hiding this comment

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

the return type of this function has to be changed to {[argName: string]: mixed}, since that's the type of call.value. not sure why flow isn't finding this.

@josephsavona
Copy link
Contributor

@xuorig see comment about the return type

@kassens
Copy link
Member

kassens commented Jun 15, 2016

I'll fix up the return type before merging this.

@facebook-github-bot import

@ghost
Copy link

ghost commented Jun 15, 2016

Thanks for importing. If you are an FB employee go to Phabricator to review.

@xuorig
Copy link
Contributor Author

xuorig commented Jun 15, 2016

Thanks @kassens !

@keithpitt
Copy link

I applied this patch locally to my copy of Relay and it fixed the problem for me. Thanks for fixing @xuorig!

@ghost ghost closed this in c9fb0e8 Jun 16, 2016
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants