Shim backburner join until we are sure Ember has it #2608
Conversation
| method = target[method]; | ||
| } | ||
|
|
||
| return method.apply(target, slice.call(arguments, 2)); |
There was a problem hiding this comment.
are u slicing args in a hot-path?
There was a problem hiding this comment.
we should also be sure not to slice beyond arguments.length - 1
There was a problem hiding this comment.
I literally copied code from backburner. Please raise the issue there :)
There was a problem hiding this comment.
There was a problem hiding this comment.
this should be fixed before merging here.
There was a problem hiding this comment.
Sure, how should I fix it? Just put a guard for argument.length? Wanna merge this asap to fix the build
There was a problem hiding this comment.
trying to get BB npm to install, i have something that should work but i need to run tests
There was a problem hiding this comment.
BackburnerJS/backburner.js#124 could use feedback
igorT
force-pushed
the
backburner-join-shim
branch
2 times, most recently
from
7538aea to
0da423a
Compare
igorT
force-pushed
the
backburner-join-shim
branch
from
0da423a to
fdf1f0a
Compare
Shim backburner join until we are sure Ember has it
This shim was added a long while back when we could not rely on the version of Backburner that Ember was using to have the `join()` function. Any currently supported version of Ember (2.x) should have this now. Similarly, any currently supported (2.x) version of Ember also has `Ember._Backburner` available. Shim was added in: emberjs#2608 `Backburner.join()` was added in: BackburnerJS/backburner.js#119
cc @rwjblue @fivetanley @hjdivad