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

[BUGFIX beta] Throw error if run.bind receives no method #16729

Merged
merged 1 commit into from
Jun 10, 2018

Conversation

Serabe
Copy link
Member

@Serabe Serabe commented Jun 8, 2018

This commit uses the same logic as backburner.join to know
which method is being bound.

Fixes #16652

@Serabe Serabe requested a review from rwjblue June 8, 2018 15:54
@Serabe Serabe force-pushed the fix/16652/run-bind-error branch 2 times, most recently from d347b61 to ae7fef6 Compare June 8, 2018 17:55
export const bind = (...curried) => (...args) => join(...curried.concat(args));
export const bind = (...curried) => {
if (!bindMethodPresent(...curried)) {
throw new Error('could not find a suitable method to bind');
Copy link
Member

Choose a reason for hiding this comment

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

Can we make this an assertion and inline the bindMethodPresent implementation in an IIFE as the predicate?

assert('Could not find a suitable method to bind.', (function() {
  // guts of conditional below...
})());

@Serabe
Copy link
Member Author

Serabe commented Jun 8, 2018 via email

@rwjblue
Copy link
Member

rwjblue commented Jun 8, 2018

I used an error because it was what was requested, but can easily be done.

Yeah, sorry about that. I had forgotten just how complicated the argument munging is for run.join (and others), and I'd rather not have to do it all twice (once for the guard, and once "for real") in production builds.

This commit uses the same logic as backburner.join to know
which method is being bound.

Fixes emberjs#16652
@Serabe
Copy link
Member Author

Serabe commented Jun 10, 2018

Done!

@rwjblue
Copy link
Member

rwjblue commented Jun 10, 2018

Thank you!!

@rwjblue rwjblue merged commit c8c96e6 into emberjs:master Jun 10, 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