-
Notifications
You must be signed in to change notification settings - Fork 8
Replace spread operator with eval() to allow safe transpilation to ES5 #4
base: master
Are you sure you want to change the base?
Conversation
const params = ['structPtr', 'selector'].concat( | ||
[].slice.apply(arguments).map((val, i) => `arguments[${i}]`) | ||
) | ||
return eval(`func(${params.join(', ')})`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so this is supposed to yield a string like func(structPtr, selector, arguments[0], arguments[1], arguments[2])
, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would something like this work?
return func.apply(undefined, [structPtr, selector, ...arguments])
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay. That'd work in a normal JavaScript environment -- in fact that's pretty much how babel transforms the spread operator iirc. But it won't work in CocoaScript as the MOBridgeSupportFunction
returned by CFunc
doesn't support .apply()
(apply
is undefined).
The eval()
I'm using here is really just a poor-mans .apply()
to pass all the supplied arguments on to the MOBridgeSupportFunction
invocation. There may be a better way to do it though?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kannonboy would it work to grab apply
from somewhere else and then call it with a MOBridgeSupportFunction
?. eval is a pretty bad hack from a perf and cleanliness perspective. Overall this library has some nasty hacks on the objc side, so I guess we can use it if all else fails.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@darknoon I agree using eval()
it's pretty far from ideal, but I think it might be the best option available.
I tried to see if I could repurpose Function.prototype.apply()
by messing around in Sketch's console, but it doesn't quite work as I'd hoped:
Function.prototype.apply.apply(
function(arg) {
log('called with ' + arg)
},
['cat']
)
Result is:
called with undefined
And the implementation of .apply()
is native of course (you can test with log(Function.prototype.apply.toString())
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Performance wise is slice bad for arguments and eval bad for variable lookup etc. See #8 for more + tested alternative solution :)
…on#1 // using babel-preset-env with targets safari 6 + Function+arguments workaround
Addresses the issue discussed in #1 & airbnb/react-sketchapp#106 (comment)
Thanks for the awesome library btw!