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

Crashes horribly when function names are invalid identifiers #20

Open
LinusU opened this issue Jun 1, 2016 · 5 comments
Open

Crashes horribly when function names are invalid identifiers #20

LinusU opened this issue Jun 1, 2016 · 5 comments

Comments

@LinusU
Copy link

LinusU commented Jun 1, 2016

See mciparelli/co-redis#16

Small test case to reproduce:

const thenify = require('thenify')

function testMe () {}

Object.defineProperty(testMe, 'name', { value: 'test-me' })

thenify(testMe)
@LinusU
Copy link
Author

LinusU commented Jun 1, 2016

Also reported here mciparelli/co-redis#16 and here redis/node-redis#1075

@jonathanong
Copy link
Contributor

it's like they are intentionally trying to break it... :P

what's your solution here?

@LinusU
Copy link
Author

LinusU commented Jun 1, 2016

Hehe, it seems that way 😆

I would actually prefer this being fixed on both ends, we could probably implement a quick check for valid names here, and if the name is invalid we could drop the name...

Another solution would be to utilize the same check that they are using, and in newer engines (e.g. Node.js >4) set the name by using Object.defineProperty instead of using eval. That way this wouldn't have blown up...

@juliangruber
Copy link

a function's .name containing - and other chars is totally valid from a language point of view, so -1 on having to change that in the redis libs.

@magicdawn
Copy link

I'm using eval to build cowrap for keeping the function.length, but what's purpose of this using eval here? keeping the function name?

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

No branches or pull requests

4 participants