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 ObservableComponent::useEvent ignoring seed #135

Merged
merged 2 commits into from
Dec 12, 2018
Merged

Fix ObservableComponent::useEvent ignoring seed #135

merged 2 commits into from
Dec 12, 2018

Conversation

Nimelrian
Copy link
Contributor

@Nimelrian Nimelrian commented Dec 12, 2018

ObservableComponent::useEvent checked whether
more than one argument was supplied (correct), but
used the 3rd argument as a seed value instead.
That argument is always undefined when using
Refract according to the docs.

This bug was introduced with commit d24f0f0.

This commit changes the function signature back
to the original one of using 2 args, one being
optional. The check for the actual argument count
is then done using the arguments object.

Fixes #134

ObservableComponent::useEvent checked whether
more than one argument was supplied (correct), but
used the 3rd argument as a seed value instead.
That argument is always undefined when using
Refract according to the docs.

This bug was introduced with commit d24f0f0.

This commit changes the function signature back
to the original one of using 2 args, one being
optional. The check for the actual argument count
is then done using the arguments object.
The arguments object cannot be used in arrow
functions. useEvent doesn't have to be an arrow
function, so we define it as a regular function
instead.
@troch
Copy link
Collaborator

troch commented Dec 12, 2018

Oh wow... Thanks for the fix. Yeah arrow functions don't have arguments.

Would replacing args[2] with args[1] work?

@Nimelrian
Copy link
Contributor Author

Nimelrian commented Dec 12, 2018

Had to do a force push to clean up the commit message, sorry about that.

Replacing args[2] with args[1] would work as well, but when the function's not really variadic in the first place, I'd rather use a regular signature and check the arguments object.

@troch
Copy link
Collaborator

troch commented Dec 12, 2018

Had to do a force push to clean up the commit message, sorry about that.

No worries, I don't mind force pushing 😂

@troch troch merged commit 4cc2548 into fanduel-oss:master Dec 12, 2018
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