-
-
Notifications
You must be signed in to change notification settings - Fork 131
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
Polyfill support for __proto__ in IE <= 10 #439
Polyfill support for __proto__ in IE <= 10 #439
Conversation
34fa94d
to
19e6ff9
Compare
19e6ff9
to
6df9e5a
Compare
tests/unit/classes/column-test.js
Outdated
let col = new Column(); | ||
assert.ok(col.someMethod()); | ||
}); | ||
|
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.
Not really sure, if these tests make a lot of sense, since we don't run CI tests for IE anyways.
Also, they mutate the Classes. This means that all following test cases have access to someStaticMethod
and someMethod
. This might not be a problem now (and I doubt it ever will), but it could and feels kind of dirty.
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.
I think, I should rather write a specialized unit test for utils/fix-proto
. This way, there's no mutation of global classes.
8d84f79
to
23be763
Compare
There's now a dedicated test for The testes for All these tests are pretty meaningless though, as we don't test in IE <= 10. But now we could verify regressions quickly, if someone reports this issue again. 🤷♂️ |
@buschtoens I think we should just move to pure Ember.Object in the next major release because this is kinda getting gross lol. |
Fixes #436.
This gives full support for stuff like this in IE <= 10:
Juicy technical details
MDN on
Object.prototype.__proto__