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

including es5-shim instead of standalone bind polyfill #121

Merged
merged 1 commit into from
Feb 27, 2014

Conversation

magalhas
Copy link
Contributor

Phantomjs bind polyfill doesn't work properly in certain cases. Also this is a good way to include scripts at the top whenever needed (such as other polyfills).

@sindresorhus
Copy link
Member

Phantomjs bind polyfill doesn't work property in certain cases

What cases? I'd rather see the polyfill fixed than adding another option.

@magalhas
Copy link
Contributor Author

Using React on Jasmine results on an invalid instanceof usage. Though making the polyfill work is a good approach, I think that being able to include other polyfills or scripts at the top is a nice to have feature.

@jsoverson
Copy link
Member

The polyfill was taken from MDN, what's the accepted best polyfill for bind()?

@jsoverson
Copy link
Member

actually, scratch that, i'm just going to include es5-shim to catch any other cases that might exist in phantom.

@magalhas
Copy link
Contributor Author

I think es5-shim solves the issue. Though being able to change which polyfills are being used would be nice. Do you want me to change the PR to include es5-shim?

@magalhas
Copy link
Contributor Author

Following up what you've said @jsoverson I've included the es5-shim dependency, It solved my particular issue and it'll probably solve many others.

@jsoverson jsoverson merged commit 476c1b6 into gruntjs:master Feb 27, 2014
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.

3 participants