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 Object.defineProperty instead of __defineGetter__ #94

Closed
wants to merge 1 commit into from

Conversation

Pumpuli
Copy link

@Pumpuli Pumpuli commented Oct 16, 2015

__defineGetter__ is non-standard and doesn't exist in IE<=10

__defineGetter__ is non-standard and doesn't exist in IE<=10
@watson
Copy link
Collaborator

watson commented Oct 16, 2015

I don't see an issue with using Object.defineProperty, but there might be some compatibility issues with that one as well that I don't know of (it's ECMAScript 5 after all). Any particular reason you chose to use __defineGetter__ originally @mafintosh? If not, I think we can just merge this 😃 Or we could consider using a polyfill if compatibility is an issue

@Pumpuli
Copy link
Author

Pumpuli commented Oct 16, 2015

Oh, I should've looked first. Now I see there's already #58, although that PR is mostly other parts and just removes the __defineGetter__ part completely. Also this doesn't apparently help IE8:

Object.defineProperty in Internet Explorer 8 only on DOM objects and with some non-standard behaviors.

https://mdn.io/defineProperty#Browser_compatibility

@Pumpuli
Copy link
Author

Pumpuli commented Nov 11, 2015

Well, #100 was merged with the exact same change so this can be closed.

@Pumpuli Pumpuli closed this Nov 11, 2015
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.

2 participants