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.

Sign up for GitHub

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

Jump to bottom

Replace hasownproperty with defineProperty #256

Closed
ghost opened this issue Jul 10, 2014 · 12 comments
Closed

Replace hasownproperty with defineProperty #256

ghost opened this issue Jul 10, 2014 · 12 comments

Comments

@ghost
Copy link

ghost commented Jul 10, 2014

HasownProperty fails in ie8 and isn't the point of this shim to get browsers without es5 to work they pretty much all have it, except for ie8.

Here is my reference.
http://phrogz.net/death-to-hasownproperty
@ljharb
Copy link
Member

ljharb commented Jul 10, 2014

Can you provide a test case, or be more specific? The point of the shim is to match the spec - that link has to do with user code. There's no choice here that I can see.

Also, Object.hasOwnProperty is non-standard - Object.prototype.hasOwnProperty is not broken in any implementation I am aware of, and that's what we're using.

@ljharb
Copy link
Member

ljharb commented Jul 15, 2014

Since Object.hasOwnProperty isn't a thing, I'm closing this for now. Please feel free to comment if you have more information.

@ljharb ljharb closed this as completed Jul 15, 2014
@ghost
Copy link
Author

ghost commented Jul 26, 2014

well you have it on 2 different lines
31 and 205.

and you are using it 13 times with the method owns()

You need to replace it with defineProperty to support ie8.

http://jsfiddle.net/scubajosh/pfxP5/626/embedded/result/

since ie8 you cant edit js fiddle you have to view it in the result.

here is how to edit
http://jsfiddle.net/scubajosh/pfxP5/626/

It shows the error in your code for your owns method. HasOwnProperty isnt needed at all please check http://phrogz.net/death-to-hasownproperty for reference.

@ljharb
Copy link
Member

ljharb commented Jul 27, 2014

Can you link me to a line directly? I'm pretty confident we're only referencing Object.prototype.hasOwnProperty. owns uses call which is using the prototype method.

Object.hasOwnProperty should never exist in any code anywhere, including yours.

Object.defineProperty does not exist in IE 8.

That particular article is from 2012, and is quite simply incorrect when you're talking about pre-ES5 browsers. To support IE 8, we must use hasOwnProperty.

@ghost
Copy link
Author

ghost commented Jul 28, 2014

@ljharb you are correct the problem wasn't in your code it was in exoskeleton js which I got working in ie8 now thanks. The other guys blog screwed me up, the death to has own property dude had no idea what he was talking about. MSDN Docs state that hasownproperty worked since ie6 and is not available in host object for ie8 and below. Thanks for your help on this much appreciated.

@ljharb
Copy link
Member

ljharb commented Jul 28, 2014

Thanks, glad to hear I'm not crazy :-)

@opsb
Copy link

opsb commented Jul 28, 2014

Sorry to keep this one going but I'm getting an error thrown in ie8 at /~https://github.com/es-shims/es5-shim/blob/master/es5-sham.js#L31. "Object doesn't support this property or method".

@ljharb
Copy link
Member

ljharb commented Jul 29, 2014

You can't use the es5-sham unless you've also used the es5-shim first. Are you?

@blimmer
Copy link

blimmer commented Aug 21, 2014

I run into the same problem @opsb was seeing (line 31 "Object doesn't support this property or method".)

I don't see this issue when I include both the es5-shim and the es5-sham after (as @ljharb ) mentioned. Is he correct that you must include both to prevent this error for IE8? If so, it might be wise to add that information explicitly to the README.

@ljharb
Copy link
Member

ljharb commented Aug 21, 2014

@blimmer if by the "same problem" you mean with ember-cli or exoskeleton, then no, that's not an issue with this shim. You should not need es5-sham to prevent errors for IE 8, unless you have a library that's incorrectly using Object.create.

However, you can't use the es5-sham without the es5-shim in a non-ES5 browser.

@blimmer
Copy link

blimmer commented Aug 21, 2014

I'm not referring to an issue with Ember.JS, though this is how I originally saw the error. I see the Object doesn't support this property or method error when running the following code in IE8:

<!DOCTYPE html>
<html>
<head>
  <script src='es5-sham.js'></script>
</head>
<body>
  <h1>Hello World</h1>
</body>
</html>

So, as you mentioned before, IE8 requires inclusion of es5-shim before the es5-sham can be included. It might be helpful to mention that in the documentation.

@ljharb
Copy link
Member

ljharb commented Aug 21, 2014

Ah, gotcha! I'll do that, thanks.

ljharb added a commit that referenced this issue Aug 21, 2014
@ljharb
Noting that es5-sham requires es5-shim, per #256 (comment)
6b3fda6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@ljharb @opsb @blimmer