This is an archived snapshot of W3C's public bugzilla bug tracker, decommissioned in April 2019. Please see the home page for more details.

Bug 28244 - Requiring @@toStringTag on instances may have performance implications
Summary: Requiring @@toStringTag on instances may have performance implications
Status: RESOLVED MOVED
Alias: None
Product: WebAppsWG
Classification: Unclassified
Component: WebIDL (show other bugs)
Version: unspecified
Hardware: All All
: P2 normal
Target Milestone: ---
Assignee: Cameron McCormack
QA Contact: public-webapps-bugzilla
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-03-19 17:41 UTC by Joshua Bell
Modified: 2017-12-06 12:23 UTC (History)
14 users (show)

See Also:


Attachments

Description Joshua Bell 2015-03-19 17:41:04 UTC
The shiny new ECMAScript binding definition of class strings calls for a @@toStringTag own property on each object instance implementing an interface, and another own property on the prototype object. This may have memory/performance implications since each object now has just a bit more data.

Over in https://code.google.com/p/chromium/issues/detail?id=239915 there's a suggestion relayed from AWB to put the logic in a getter on the prototype, e.g.

Object.defineProperty(HTMLDivElement.prototype, Symbol.toStringTag, {
  get: function() {
    if (this === HTMLDivElement.prototype) {
      return 'HTMLDivElementPrototype';
    }
    return 'HTMLDivElement';
  },
  configurable: ???,
  writable: ???
});

Thoughts on revising WebIDL's definition here, before implementations bake anything in?
Comment 1 Boris Zbarsky 2015-03-19 17:50:50 UTC
> calls for a @@toStringTag own property on each object instance implementing an
> interface

This is just because we want different toString() results for Node instances and Node.prototype, right?

I agree that putting this on all instances is totally undesirable.

Hanging it off the prototype with an accessor means that the Object.prototype.toString value changes if you munge the proto chain.  Maybe that's OK.

The pseudocode in comment 0 doesn't work that well for cross-origin invocation of various sorts, but that could be solved in spec prose.

In some ways, what would be ideal here is having a more extensible way to hook into toString via branding, the way ES6 does for the objects _it_ defines....  Certainly that's how DOM toString stuff is implemented right now and has been for a long time, and I have some web compat worries about changing it. :(
Comment 2 Erik Arvidsson 2015-06-04 15:45:30 UTC
I think we should just have a data property on the prototype. The spec currently says that the Object.prototype.toString.call(Foo.prototype) should return "[object FooPrototype]". This legacy requirement is not needed for the web. (I assume this behavior came from Gecko's XPCOM bindings?) and Chrome never supported this. Chrome has always returned "[object Object]" for the prototype object.

My suggestion is to follow ES6 and just add a data property for the @@toStringTag to Foo.prototype with the value "Foo".

https://people.mozilla.org/~jorendorff/es6-draft.html#sec-map.prototype-@@tostringtag

This would mean that we get:

Object.prototype.toString.call(document.body) === '[object HTMLBodyElement]'
Object.prototype.toString.call(HTMLBodyElement.prototype) === '[object HTMLBodyElement]'
Comment 3 Domenic Denicola 2015-06-04 16:09:02 UTC
I agree with Arv. It is consistent with the ES built-ins, and more performant, and simpler. Given that Chrome provides evidence that it is not a compatibility issue, we should just do the simple thing.
Comment 4 Boris Zbarsky 2015-06-04 16:45:09 UTC
> Given that Chrome provides evidence that it is not a compatibility issue

Sadly, not quite.  Chrome provides evidence that returning "Object" as the string tag for HTMLBodyElement.prototype is not a compat issue.  Returning "HTMLBodyElement" would break code that tries to work around cross-global instanceof not working in some browsers by checking Object.prototype.toString.call().  Doubly so because all of the methods/getters one would want to call on a <body> exist on HTMLBodyElement.prototype but throw when called.

I'm game to try the simple thing in theory, but in practice I worry about compat issues.  If we have data showing there is no issue (e.g. an implementation shipping the proposed behavior for a bit, or some other indication that people are not using Object.prototype.toString.call() in the way I worry about), I'd love to see it, now or in the future.
Comment 5 Erik Arvidsson 2015-06-04 16:47:09 UTC
My goal is to try to make this change in Blink. I will report back on possible issues.
Comment 6 Joshua Bell 2016-02-26 17:38:45 UTC
We appear to be finally trying out the experiment (Object.prototype.toString.call(HTMLBodyElement.prototype) === '[object HTMLBodyElement]') in Chrome M50 which enters Dev shortly. We'll see if it sticks and report back here.
Comment 7 Domenic Denicola 2016-08-11 02:30:44 UTC
FYI this stuck in Chrome and Firefox is intending to follow suit: https://groups.google.com/forum/#!searchin/mozilla.dev.platform/toStringTag%7Csort:relevance/mozilla.dev.platform/IZNh8QAXkFA/me59gpo5PgAJ We should change the spec as soon as feasible, to just a data property on the prototype.
Comment 8 Boris Zbarsky 2016-08-11 03:07:24 UTC
It's not clear what Firefox will be doing here, if anything.  There was pushback to the intent to ship on general grounds of web developer confusion and there are unclear issues around compat.  I personally have put the whole thing on indefinite hold for the moment and have no plans to pick it up.
Comment 9 Philip Jägenstedt 2016-08-11 09:28:22 UTC
Mike Smith pointed me here. Does someone have a test that could show the different behavior in all engines? Trying to understand if there's already a majority behavior.
Comment 10 Michael[tm] Smith 2016-08-11 10:03:20 UTC
(In reply to Philip Jägenstedt from comment #9)
> Does someone have a test that could show the different behavior in all engines?

    (new Object()).toString.call(StorageEvent.prototype)

Per the current WebIDL spec that should give you "[object StorageEventPrototype]". And it does in Gecko and WebKit and (I think) in Edge.

But in Blink it instead now gives you "[object StorageEvent]"
Comment 11 Boris Zbarsky 2016-08-11 15:05:31 UTC
Indeed, what comment 10 says.  And before Blink changed its behavior it used to give "[object Object]" in Blink.
Comment 12 Philip Jägenstedt 2016-08-29 12:19:08 UTC
I can confirm that Edge has the same behavior as Gecko here:
(new Object()).toString.call(StorageEvent.prototype) === "[object StorageEventPrototype]"
(Object.prototype.toString.call(HTMLBodyElement.prototype)) === "[object HTMLBodyElementPrototype]"

Where did things land in terms of how it's implemented?

https://crbug.com/239915 was closed as fixed with a different behavior, so adding Blink bindings team lead haraken@ to see if this can be prioritized. (Not sure if there's an open issue.)
Comment 13 Domenic Denicola 2016-09-02 16:53:41 UTC
It would be really good to work out the story here.

I strongly believe that @@toStringTag on the prototype, as a simple data property instead of a getter, is more correct:

- It matches the ES built-ins
- It fits better with JavaScript's prototypal inheritance model (now that toString behavior is something that prototypally inherits)
- It's simpler
- It's possibly more performant (although a sufficiently smart optimizer could probably equalize them)
- It's very likely to be web-compatible given that it's been shipping in Chrome for several releases

Per comment #8, Boris doesn't quite agree, or at least doesn't want to be second-mover here.

Perhaps we could get other engines to comment? In particular, have any of them started moving their DOM to use ES6 @@toStringTag instead of ES5 [[Class]], such that they have an opinion on this matter?
Comment 14 Boris Zbarsky 2016-09-02 17:39:39 UTC
> Per comment #8, Boris doesn't quite agree

Comment 8 was about what I'm doing (nothing, partly because I don't have time to pursue a change here in the face of probably reasonable opposition and partly because I have 0 interest in making changes that I will then have to revert because other UAs don't follow), not what I'm thinking.

If you want to know what I'm thinking...

> - It matches the ES built-ins

I think, personally, the ES built-ins are broken.  The setup kinda sorta made sense for the ES3 builtins where the prototype was really an instance too (Array, Date).  It's pretty nonsensical, imo, for the new built-ins ES6 adds (Map, Set, Promise, etc) where all the things you might want to do with an instance throw if you do them with the prototype.  Now I'm obviously not going to change TC39's mind at this stage in the game, but I think they made a mistake here.  The only saving grace is that at least Map.prototype instanceof Map tests true by default.  But in a multi-global world instanceof is just broken, so you end up with Array.is (but refusals to add it for anything else) and people being told they should not worry about validating their inputs and instead just deal with exceptions if/when they arise.  Except they don't, and you get broken websites when some idiot library walks the object graph and suddenly finds an object it did not expect; run into that a number of times as browsers have added new features.  So people who want to avoid that pitfall and want to be careful use Object.prototype.toString to see what they're really working with, since instanceof is useless for that purpose.  Of course with @@toStringTag this method of telling what you have might also become useless... we'll see.

I know this is getting rather far afield, but the theory of how people write JS and the practice of how at least a large minority do so are very much at odds, and it's the theory that's been driving TC39 decisions...  As a result, I'm biased to be somewhat sceptical of adopting those decisions wholesale in other parts of the web platform, particularly when that involves changing long-standing behavior; it's bitten us before when we have.

>- It fits better with JavaScript's prototypal inheritance model (now that toString behavior is something that prototypally inherits)

I don't see how using an accessor property fails on this bullet point, for what that's worth, any more so than any other thing living on the prototype that only works on instances; ES6 is littered with that sort of stuff, as are the IDL-defiend parts of the web platform.

>- It's simpler

Granted.  If we had no history here and had some other way of doing brand checking, I don't think anyone would even dream of suggesting that we should use Object.prototype.toString as a brand checking mechanism.  Objectively speaking, it is, of course, daft.

>- It's possibly more performant (although a sufficiently smart optimizer could probably equalize them)

Yes on both counts.  I question the performance sensitivity of Object.prototype.toString on the level we're talking about here, btw; just the string concatenations required for it already involve a bunch more work than either option here, I expect.

>- It's very likely to be web-compatible given that it's been shipping in Chrome for several releases

Maybe you meant this is the part I don't agree on?

My experience is that in today's market Chrome can get away with breaking sites when other browsers cannot; people will work around Chrome issues and just leave their sites broken in other browsers.  Often enough the workaround involves sniffing for Chrome and doing something different there, so even if other browsers implement exactly what Chrome does the site will _still_ be broken in them.  This is unfortunate, but it does mean that "Chrome shipped it" is not as useful an indicator of web compat as it could be.  It's better than nothing, of course, which is why this discussion is happening at all.

I, personally, happen to think this is web compatible enough, that it just screws over people trying to be careful, and that most people writing code on the web are not being very careful in this way and hence will be unaffected.  I was not able to sell others within Mozilla on this viewpoint, though, at least not with the limited amount of effort I put into it.

> Perhaps we could get other engines to comment? 

That would be the most useful thing, yes.

P.S. Note that Chrome's devtools most definitely display event instances and Event.prototype differently; they are clearly making a branding decision somewhere in there.  How they make it given the relative paucity of public facilities for this sort of thing, and why we want to deny web pages the ability to make similar distinctions, is an interesting question.
Comment 15 Domenic Denicola 2016-09-02 17:43:31 UTC
Just to be clear, I sympathize with your desire for a better branding story. But I don't think making the behavior of @@toStringTag different from the ES built-ins is the right way to accomplish this. (I feel similarly for @@hasInstance; see https://github.com/heycam/webidl/issues/129.)
Comment 16 Boris Zbarsky 2016-09-02 17:46:45 UTC
Sure, see the "no history" paragraph.  The compat concern is whether in the face of a lack of better branding story people are using toString as poor-man's branding in ways that would break if this change got made.  The fact that Chrome shipped the behavior it did is some indication that maybe in practice they are not, I agree.
Comment 17 Philip Jägenstedt 2016-09-30 10:32:12 UTC
Do we have a path forward here? As long as there is divergence, one engine or another will see a lot of noise in idlharness.js tests in web-platform-tests, making it harder to spot real issues (more tooling would help, we're trying that too).
Comment 18 Rick Byers 2016-12-16 18:38:36 UTC
> As long as there is divergence, one engine or another will see a lot of noise in idlharness.js tests in web-platform-tests, making it harder to spot real issues (more tooling would help, we're trying that too).

+1.
I haven't fully parsed the whole history here, but I agree getting consensus on this is important to IDL tests.  Is there any work / data the Chrome team could provide that would help with the compat argument?
Comment 19 Domenic Denicola 2016-12-16 18:47:35 UTC
It's worth noting that nobody seems to actually follow the spec here. That is, Object.getOwnPropertySymbols(document.createElement("div")).length is 0 in all browsers I've tested. (I haven't yet tested Safari.)

Given that, I think we should at the very least remove the mention of class strings and @@toStringTag from Web IDL and from testharness.js. Or we could align with Blink's implementation, which at least is implemented in terms of @@toStringTag, whereas nobody else seems to have updated to ES6 at all.