cujojs/meld

usage defineProperty in ie8

Closed this issue · 10 comments

defineProperty usage here: https://github.com/cujojs/meld/blob/master/meld.js#L161

It seems that ie8 implements defineProperty differently and it is not easily shimmable: https://github.com/kriskowal/es5-shim

Could this be replaced by getters/setters instead?

Hey @Badunk,

If you look at line 56, you can see that it is shimmed. :)

Unfortunately, getters and setters are even harder (impossible?) to shim in IE8. We've tried in our polyfill project. :(

@briancavalier: why are we using defineProperty, anyways? Just dreaming of the future when legacy IE is truly dead? :)

-- J

oh neat

In that case, I think the problem is that ie8 actually does have it defined, so it doesn't go to the fallback, however its definition has a different signature that only accepts DOM objects.

Ah ha, yep. This is a problem in IE8. Thanks for finding this, @Badunk!

We're using defineProperty in a couple places so we can make constructor and _advisor non-enumerable and/or non-writeable, etc.

But yeah, we should test for a working Object.defineProperty rather than just its presence to avoid IE8 problems! Thanks @Badunk, we should be able to get in a quick fix soon.

Candidate fix in e1281a9

Let me know if that looks good to you guys. Thx!

@Badunk do you have a scenario where this was causing a problem for you? If so, please try out the dev branch and let us know if it helps. Thanks!

I have just tested this. It appears to break at a different place now after the fix and only because I am using the constructor advice. Ie8 doesn't support Object.create - I did find some tips here

Thanks for the quick turnaround!

@Badunk Cool, thanks for testing it. It sounds like the immediate issue of Object.defineProperty in IE8 is fixed.

As for Object.create, yeah, see the docs here. For now, we support constructor advice in envs where Object.create or a polyfill is available. Is using either poly or es5-shim an option for you?

It does seem a little inconsistent that meld internally shims Object.defineProperty, but doesn't shim Object.create. We should probably account for both, or neither :) I'll mull that over!

problem solved! The shims work for me, I forgot about that again when I tried to get it to work again.

Thanks

Great, glad to hear the shims did the trick.