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

Use of named function expressions can cause scoping issues in some implementations #12

Closed
ckknight opened this issue Mar 28, 2011 · 8 comments

Comments

@ckknight
Copy link

In a recent commit, many functions have been changed to be defined as named function expressions, e.g. var bound = function bound(). One of the issues that arises from this is that in JScript (IE), var bound and function bound create two separate functions.

See http://kangax.github.com/nfe/ for more details about this.

@Gozala
Copy link
Contributor

Gozala commented Mar 28, 2011

Thanks @ckknight for pointing that out, I did not knew about that bug. Still as bound with in the bound function is only used for instanceof it will work as expected since both functions will have same prototype. Also I guess it's probably better and safe to remove var bound = entirely as it's not really necessary anyway. Also all other cases seem are fine as named functions are just saved as properties

@ckknight
Copy link
Author

The safest way is to either use the anonymous function expression or to use a proper function declaration. They're essentially equivalent (aside from the hoisting).

@Gozala
Copy link
Contributor

Gozala commented Mar 28, 2011

On Monday, 2011-03-28 at 09:25 , ckknight wrote:
The safest way is to either use the anonymous function expression or to use a proper function declaration. They're essentially equivalent (aside from the hoisting).

I believe my suggestion to remove var bound = would make a proper function declaration. Do you mean we need to do more then that ?

Reply to this email directly or view it on GitHub:
#12 (comment)

@ckknight
Copy link
Author

Nope, just go with either var bound = function() { } or function bound() { }.

@ckknight ckknight reopened this May 23, 2011
@ckknight
Copy link
Author

Apparently I accidentally closed this. This is still an issue.

@michaelficarra
Copy link
Member

I don't see how this is actually a problem. If it was var boundA = function boundB() { ... }, I could see an issue, but it's not broken the way it is. Still, I guess it's better to just change it.

@ckknight
Copy link
Author

It's minimally a memory leak in JScript (IE), since it creates two function instances rather than one.

Gozala added a commit to Gozala/es5-shim that referenced this issue May 24, 2011
@Gozala
Copy link
Contributor

Gozala commented May 24, 2011

Change above fixes this issue. closing!

@Gozala Gozala closed this as completed May 24, 2011
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

3 participants