tc39 / how-we-work

Documentation of how TC39 operates and how to participate
199 stars 23 forks source link

Add override mistake #137

Closed Jack-Works closed 12 months ago

Jack-Works commented 1 year ago

LGTM but please put semicolons in your code samples.

LOL code style war, but I'll add it. Do I also need to change ' to "?   

Jack-Works commented 1 year ago

LGTM but please put semicolons in your code samples.

I think we also need to wait for people from ses to review this. cc @erights

phoddie commented 1 year ago

With all due respect, there is not consensus that this behavior is a mistake. Is it really appropriate to include it a guiding document like "how we work"?

ljharb commented 1 year ago

@phoddie i was under the impression that whenever it's been discussed in committee, it was pretty unanimous that we all wanted the behavior changed, but acked that it wasn't feasible. Either way, this is the term by which the behavior's been referred to many times, so it seems appropriate to use that term in the terminology list.

phoddie commented 1 year ago

it was pretty unanimous that we all wanted the behavior changed

I won't dispute that. However, had there been a call for consensus, Patrick or I would have expressed Moddable's position. The behavior does create compatibility issues. but that does not make the behavior undesirable in all situations. (I don't want to debate the point here, only ensure that this document revision is fair.)

Either way, this is the term by which the behavior's been referred to many times, so it seems appropriate to use that term in the terminology list

The name of the term is unfortunate, as it takes a position on the behavior. The text that follows could correct that.

ptomato commented 1 year ago

I'm not sure that's the purpose of this document. "The override mistake" is a term that is in fairly common use within TC39 and the ship has probably sailed on renaming it. There's no "fairness" or "unfairness" in defining it in here. What I'd like to see from this document is defining terms that are in common use so that people who aren't familiar with the jargon have a go-to place to look it up. Particularly to be welcoming to new delegates, but also I myself always have to go look at my notes when to figure out what the override mistake was because its definition isn't top of mind for me either.

Add a sentence like "Not everyone agrees it was a mistake" if you like, but please let's not hold up this newcomer-friendliness improvement on that discussion.

bakkot commented 1 year ago

Adding sentence like "There is not universal agreement that this is a mistake, but enough delegates have gotten used to using this term that we're stuck with the name anyway." SGTM

michaelficarra commented 1 year ago

More like "there is near-universal agreement", but sure.

phoddie commented 1 year ago

@ptomato, Please understand I am not raising an objection to including this information in the document. I support your goal of helping more people understand more of TC-39's discussions. I've been through that learning curve and it isn't easy. Still, the name of this term is not a neutral summary of the technical topic. While it may be too late to change the name, the text can and should correct that.

@bakkot, thank you for the suggestion. "There is not universal agreement that this is a mistake" would address my concerns.

@michaelficarra and @ljharb, I am resigned to the reality that Moddable's advocacy for JavaScript issues from the perspective of embedded uses can be at odds with web orthodoxy. Still, I don't appreciate comments that further marginalize the input. While I surely represent a minority, that doesn't automatically make the input incorrect or irrelevant.

michaelficarra commented 1 year ago

@phoddie I'm not trying to minimise your input, but I would prefer it be accurately represented as a small minority. @bakkot's suggested wording, in my opinion, overstates the "controversy" and over-represents this (entirely valid, but very minority) opinion. You and I both know that, if we took a straw poll at any plenary, I would be able to count on a single hand the number of delegates that did not consider this to be a mistake.

To your point about the term itself being problematic because it is judgmental, I am fully sympathetic, but the term is too ingrained in our culture at this point to do anything about it. A note explaining the non-universality of this particular judgment seems a more-than-sufficient compromise to me.

bakkot commented 1 year ago

"Not universal" was carefully chosen to make no claims about the level of agreement except that it is less than 100%. I don't think there's much value for this document in particular in trying to nail down the precise level.

bakkot commented 1 year ago

Disagree. This is a glossary: it should document the terms people actually use. If at some point in the future we switch to using a different term, we can document that one too. But right now this a term people are going to encounter in discussion, and they should be able to use this document to understand that discussion.

(Or are you just objecting to the claim that we can't do anything about it? I'm fine with leaving that part out and saying only "There is not universal agreement that this is a mistake", sure.)

ljharb commented 1 year ago

@phoddie it's a bit disingenuous to claim we're marginalizing an opinion that's never been shared before, in plenary or elsewhere. If Moddable has an argument for why the "override mistake" is less than "catastrophically bad", I suspect we'd all love to hear it - but it's not been offered yet afaik.

Either way, the purpose of this list is to document terms people use, and if the term people use editorializes, so be it.

Jack-Works commented 1 year ago

With all due respect, there is not consensus that this behavior is a mistake. Is it really appropriate to include it a guiding document like "how we work"?

We can also document the good intent behind this behavior (but I don't know what it is. It's an ES5-era thing). Does anyone know why it was designed in this way in the first place?

mhofman commented 1 year ago

Does anyone know why it was designed in this way in the first place?

Here is a likely relevant quote: https://github.com/tc39/ecma262/pull/1307#issuecomment-421094561

and some more background: https://github.com/tc39/ecma262/pull/1307#issuecomment-432822787

hax commented 12 months ago

I also wouldn't necessarily consider this to be a "mistake".

If my memory and understanding serve me right, when ES5 introduced getters/setters, it aimed to ensure consistency with the behavior of writable: true, leading to this design. If there was a mistake to be pointed out, perhaps we could argue that the root issue may lie in the design of writable itself. In the presence of an accessor, writable is actually redundant. The coexistence of both writable and accessors might because, at that time, some browsers implemented read-only DOM properties not with accessors, but with own properties set to writable: false. However, today, all DOM APIs are based on accessors.

ljharb commented 12 months ago

writable and get can't exist in the same property descriptor.

bakkot commented 12 months ago

OK, I think it's pretty well established that

Given that this is a glossary, I don't think we need to try to document the history or all of the pros and cons of the behavior. I do think it's worth mentioning that it has caused back-compat issues, since that's the place you're most likely to run into the term.

@Jack-Works can you update this PR to add "There is not universal agreement that this is a mistake" (maybe with a link to this comment), and to change the "to fix this" bit to instead be "to change this", and then we can land it?

hax commented 12 months ago

writable and get can't exist in the same property descriptor.

@ljharb

What I mean is the functionality of writable: false data property and the getters are originally used for the same feature (DOM APIs) in early days, if in that time browsers agreed to unified DOM APIs to accessors like today, TC39 didn't need to introduce writable attribute in the first place, then no "override mistake".

phoddie commented 12 months ago

There seems to be a misunderstanding that I proposed changing the unfortunate name assigned to this behavior. I have not. I have asked for clarifying text and supported the sentence proposed by @bakkot for that.

@michaelficarra – I am dismayed that you responded my request not to further marginalize minority input by seeming to do just that, requesting that the position be described as held by not just a minority but a "small minority" and noting that supporters could be "[counted] on a single hand."

@ljharb – There was nothing "disingenuous" about my statement that there is not consensus on this topic. The links @mhofman unearthed from Allen Wirfs-Brock establish that. The details of Moddable's position are not important to the request to add clarifying text regarding the name.

ljharb commented 12 months ago

@phoddie I don't believe that opinion was ever shared in plenary, only on github; the last time it was discussed in plenary I don't recall anything but belief that the behavior is a mistake.

It's of course fine to add a statement that not everybody believes it's a mistake; but nonetheless that's the term that's used to describe it, and thus that's the term that belongs in this document, full stop.

ctcpip commented 12 months ago

@phoddie @hax are you comfortable with the PR content as currently written?

phoddie commented 12 months ago

@ljharb - Perhaps you overlooked the beginning of my comment, "There seems to be a misunderstanding that I proposed changing the unfortunate name assigned to this behavior. I have not."? That's the only explanation I can see for this: "but nonetheless that's the term that's used to describe it, and thus that's the term that belongs in this document, full stop."

@ctcpip – the change is fine. I like that it includes a link to the earlier comment, as that will help people's understanding. Regarding "consensus" versus "universal consensus".... in TC39 we use "consensus" to mean universal (at least no blocking objections) but the actual definition of the word "consensus" does not imply universal. I'm OK with either here.

ljharb commented 12 months ago

the name of this term is not a neutral summary of the technical topic. While it may be too late to change the name, the text can and should correct that.

@phoddie perhaps i did indeed misunderstand; if the name is fine, would a sentence such as https://github.com/tc39/how-we-work/pull/137#issuecomment-1724461368 suffice to correct that? (edit: i see your recent comment which implies that it would, which is great!)

(either way, I don't think the definition of terms needs to be neutral; this list exists to explain something, not to state a position or the lack thereof)

phoddie commented 12 months ago

@ljharb – Two days ago I expressed support for the wording proposed by @bakkot .

(either way, I don't think the definition of terms needs to be neutral; this list exists to explain something, not to state a position or the lack thereof)

The definition should be accurate while fairly and concisely representing the membership of the committee. It is a disservice to those consulting this glossary otherwise. Here the name is regrettably not neutral so it is important that the definition address that as the current draft does.