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

ES Symbols break classes #13796

Closed
chrisjshull opened this issue Jul 6, 2016 · 11 comments
Closed

ES Symbols break classes #13796

chrisjshull opened this issue Jul 6, 2016 · 11 comments

Comments

@chrisjshull
Copy link

const sym = Symbol('foo');
const Class = Ember.Object.extend({
    [sym]: 1,
    bar() {}
});

alert(Class.create().bar);

https://ember-twiddle.com/f2fa721c2a2e60ee3baed80c7de66e13

This alerts "1" in Safari (9.1) with the web inspector closed, and "function bar() {}" with the web inspector open. Chrome (correctly) always alerts "function bar() {}".

@rwjblue
Copy link
Member

rwjblue commented Jul 6, 2016

Ummm. WAT.

This seems like a browser bug to me...

@chrisjshull
Copy link
Author

This seems like a browser bug to me...

If so, would be good to get a reduction. Unfortunately, I'm not familiar enough with the Ember stack to track down a Heisenbug like this. Perhaps someone can help?

@rwjblue
Copy link
Member

rwjblue commented Jul 7, 2016

Agreed. We need to figure out what is going on here.

While randomly poking at it, I noticed that this does alert the right value:

const sym = Symbol('foo');
const Class = Ember.Object.extend();

alert(Class.create({
    [sym]: 1,
    bar() {}
}).bar);

@rwjblue
Copy link
Member

rwjblue commented Jul 7, 2016

Thanks to @krisselden, here is a minimal reproduction:

var sym = Symbol('foo');

var args = {
    [sym]: 1,
    bar() {}
};

function foo(obj) {
    var arr = [];

    for (var key in obj) {
    arr.push(key);
    arr.push(obj[key]);
    }
  return arr;
}

alert(foo(args));
var c = 30;
while (c--) {
  foo({c: c});
}
alert(foo(args));

when the function JITs, the for in breaks

@krisselden
Copy link
Contributor

krisselden commented Jul 7, 2016

@chrisjshull this is fixed at least in WebKit nightly, Safari 9.1 uses over a year old WebKit version. This does not seem like something we can workaround. JITed code that reads objects with Symbols in Safari seems fatally buggy.

What I don't understand is why do you have a symbol in there anyway? They are not enumerable and therefor ignored by create().

@danmcclain
Copy link
Contributor

Was testing in the latest Safari Tech preview, and this still seems to be an issue (opening the dev tools works). Went for the heck of it and tried with the latest WebKit nightly, and saw that this is not fixed in the nightly (or if it was, it has regressed)

js bin - collaborative javascript debugging 2016-07-07 09-54-36
monosnap 2016-07-07 09-57-16

@chrisjshull
Copy link
Author

What I don't understand is why do you have a symbol in there anyway? They are not enumerable and therefor ignored by create().

Ah, perhaps I over-reduced the problem. I originally hit this when doing a reopenClass, like this: https://ember-twiddle.com/176c808569ddd3c1ad7e35dfc4712c3e

Since you brought it up though:

  • if symbols shouldn't work thru a .create(), why is the test case working fine in Chrome?
  • is it documented somewhere that non-enumerable types aren't supported?
  • shouldn't lack of support for this be a bug in itself? plain ES classes support it:
const sym = Symbol('foo');
class Foo {
  [sym]() { console.log("hello"); }
  public() { this[sym]();  }
}

new Foo().public(); // logs "hello"

(let me know and I'll file follow on issues)

@pixelhandler
Copy link
Contributor

@chrisjshull @rwjblue this is not an Ember specific issue but a broader issue for Symbol use correct? If so should this be closed once the issue is opened up with Webkit bug tracker? https://bugs.webkit.org

@krisselden
Copy link
Contributor

It is a Safari issue, WebKit works now. And this has nothing to do with Ember.

@Serabe
Copy link
Member

Serabe commented Jul 10, 2016

Thank you all for your efforts on this issue. Since this is not an Ember issue, I'm closing it.

Thank you all!

@Serabe Serabe closed this as completed Jul 10, 2016
@chrisjshull
Copy link
Author

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

No branches or pull requests

6 participants