search

tc39 / proposal-optional-chaining

https://tc39.github.io/proposal-optional-chaining/
4.94k stars 75 forks source link

Updating spec text #73

Closed dusave closed 5 years ago

dusave commented 5 years ago

This update attempts to simplify this proposal. It:

Mouvedia commented 5 years ago

Changes the return from automatic undefined to null/undefined based on value.

Could you link the issue related to this change?

jridgewell commented 5 years ago

Thanks for making a PR!

littledan commented 5 years ago

Could you explain the rationale for arriving at these choices? At this point, I am somewhat skeptical of the null/undefined change, personally.

zenparsing commented 5 years ago

I also have a feeling the null/undefined change will be controversial. I thought immediately of how this change might interact poorly with parameter defaults (described briefly at https://github.com/tc39/proposal-optional-chaining/issues/69#issuecomment-413344785).

MatthiasKunnen commented 5 years ago

@Mouvedia, the issues would be #69 and #65. Since both discussions favor NOT returning null, I'm wondering why this decision was made. In my opinion it makes this operator more confusing. It also isn't in line with most other languages, see https://github.com/tc39/proposal-optional-chaining/issues/69#issuecomment-409045384.

Zarel commented 5 years ago

Both #69 and #65 overwhelmingly favor the automatic undefined value, in votes:

65 has 32 thumbs-down to 9 thumbs-up

69 has 34 thumbs-down to 4 thumbs-up

A poll linked from Reddit's /r/javascript has similar results:

image

Zarel commented 5 years ago

Here's a consolidated table (using data from @noppa, @hax, and @claudepache's comments in #69) on what current libraries do, with inputs of obj = {a: null} and obj = {a: undefined}:

Library Test Result
Underscore underscore.property(['a', 'b'])(obj) always undefined
Lodash lodash.get(obj, 'a.b') always undefined
Ramda R.path(['a', 'b'])(obj) always undefined
Closure library goog.object.getValueByKeys(obj, 'a', 'b') always undefined
Idx idx(obj, _ => _.a.b) obj.a
Ember Ember.get(obj, 'a.b') always undefined
Immutable Immutable.getIn(obj, ['a', 'b']) always undefined
Chai pathval.getPathValue(obj, 'a.b') always null
CoffeeScript obj.a?.b always undefined
Angular obj.a?.b always null

Idx is the only one of this list that returns null or undefined depending on if obj.a is null or undefined.

Notes:

ljharb commented 5 years ago

@Zarel to be clear, all of these accept both null and undefined as input, but only one of them potentially outputs both null and undefined?

Zarel commented 5 years ago

@ljharb Yes, your interpretation is correct. I've edited my comment to be slightly clearer.

Mouvedia commented 5 years ago

idx is not popular enough to deserve that change. Let's just compare it to the module separated from lodash; as of today:

If you consider that most ppl either use lodash (18M+) or underscore (6M+) and a bundler for tree shaking you realize that the familiarity wouldn't be the criterion that would support to pick idx's way.

Why did underscore/lodash pick always undefined? And does it really matter? IMHO we should only take into account Coffeescript and Angular* which are real prior art: you shouldn't compare oranges to apples.

Also what @zenparsing pointed out earlier is a strong argument against null being returned if the property/method doesn't exist.

* #58

littledan commented 5 years ago

From a mental model perspective, it makes sense that so many of these libraries chose undefined, as that's what you get when a property is missing. It makes sense to me to treat nullas something that's sort of like an object that is missing all of its properties, when you apply ?. to soften the blow.

dusave commented 5 years ago

Removing optional function execution is mainly due to discussions around semantics and grammatical inconsistencies that have stalled this proposal in the past. By scoping this down to the primary use case of object property access, we can get a crucial feature added while we evaluate better options and viability of optional function execution at a later time.

As for the change of null/undefined, this has been a historically controversial point of this proposal to say the least. There are a couple of reasons I made this change:

Thanks to everyone for voicing their opinions on the matter 😄

ljharb commented 5 years ago

It’s worth noting that either choice will be controversial; there are those of us that feel strongly about having the null/ undefined semantics.

littledan commented 5 years ago

@dustinsavery It'd help if you could draw these conclusions on the issue threads where the topics are being discussed, to engage the people there.

Removing optional function execution is mainly due to discussions around semantics and grammatical inconsistencies that have stalled this proposal in the past. By scoping this down to the primary use case of object property access, we can get a crucial feature added while we evaluate better options and viability of optional function execution at a later time.

The scoping down here will not meet all strong opinions that have been expressed in TC39, unfortunately. However, I think the switch to null will be something that many committee members are strongly opposed to. We can discuss this further offline if you'd like.

Mouvedia commented 5 years ago

By scoping this down to the primary use case of object property access, we can get a crucial feature added while we evaluate better options and viability of optional function execution at a later time.

I would go even further and support only ?.. I know we need some form of ?[] but what is currently proposed is controversial. Just ?. would be a slam dunk.

zenparsing commented 5 years ago

@littledan

The scoping down here will not meet all strong opinions that have been expressed in TC39, unfortunately.

Do you mean there are still strong opinions with respect to the remaining scope (e.g. ?.[)?

dusave commented 5 years ago

It’s worth noting that either choice will be controversial

I agree, there's no way to please everyone. The undefined approach seems to make the most sense to people and would cause the least cases of unexpected/unintended behavior.

I would go even further and support only ?.

I strongly considered this. The only reason I kept the bracket syntax is because the use case that this proposal is targeting felt incomplete without it. However, I would like to see ?. get through smoothly, so if it would make more sense to remove that piece, I can do so.

jackkoppa commented 5 years ago

@dustinsavery, @Mouvedia - could someone point me to any issues where ?.[] is pointed out as also controversial/difficult to implement? I, too, would be happy to drop that if it meant ?. can progress

dusave commented 5 years ago

@jackkoppa It's spread across a couple of issues: #59, and the original debate about all syntax options: #51

ljharb commented 5 years ago

@jackkoppa @Mouvedia optional member access without optional bracket access is an incomplete feature that is highly unlikely to progress.

Zarel commented 5 years ago

@dustinsavery @Mouvedia The committee rejected dropping ?.[] in March:

https://github.com/tc39/proposal-optional-chaining/pull/48#issuecomment-377612223

The committee seems to have vetoed every popular option, leaving us in a rather tough place.

dusave commented 5 years ago

@Zarel

The committee rejected dropping ?.[] in March

Just to clarify, they rejected dropping optional bracket access I believe, not the syntax specifically.

littledan commented 5 years ago

Do you mean there are still strong opinions with respect to the remaining scope (e.g. ?.[)?

Yes, I believe @ljharb had some concerns with it, if in conjunction with ?.

ljharb commented 5 years ago

My understanding is that each concern is shared by multiple committee members, such as the following (some of these I share, some I do not)

I'm sure I've missed some, and I'm happy to edit if anyone thinks I've mis-stated any of the concerns.

Mouvedia commented 5 years ago

@jackkoppa #5 but @dustinsavery is right it's spread on several issues.

jridgewell commented 5 years ago

symmetry between the tokens is imperative, ie, <something>., <something>[, <something>(

I think you were the one to say it’s necessary. 😜

I’m fine if it’s ?. and ?.[, as long as ?.( isn’t a thing.

ljharb commented 5 years ago

@jridgewell i was, but I’m not the only one who thinks that - just the only one to have spoken up :-)

noppa commented 5 years ago

@dustinsavery Removing optional function execution is mainly due to discussions around semantics and grammatical inconsistencies that have stalled this proposal in the past. By scoping this down to the primary use case of object property access, we can get a crucial feature added while we evaluate better options and viability of optional function execution at a later time.

@jridgewell opposes ?.( as the syntax for optional function calls.
@ljharb has expressed the opinion that all the operators need to share the same base (comment in #59).

If this proposal progresses in its current form, how likely is it that we'll ever see optional function calls, given that both of those requirements can't be fulfilled if ?. is chosen as the "base" for this operator?

I wouldn't personally mind if we didn't get optional call expressions, but it's important that everyone is on the same page about the finality of this change. Are we temporarily scoping down the operator or is this, in fact, a nail in the coffin for optional calls?

dusave commented 5 years ago

Given the history and debate around this proposal, my goal is to get the pieces of least debate through that have the most impact. While I would love to see all of it get through, optional chaining for property access would have, by far, the most use.

As for optional function execution, I understand that not everyone is on board with the semantics. However from my perspective if we go down the path of re-opening the syntax to try ?? or other ideas, there’s the potential to lose ground and prolong (possibly indefinitely) this proposal from reaching stage 4.

hax commented 5 years ago

I am so happy that we can have some progress finally!

I agree optional function call is not very useful and even a little bit confusing. There are also many cases you really want typeof a.b === 'function' && a.b() which not match current semantic.

About a?.[prop], it's a little inconsistent, but in practice, many programmers never notice such inconsistency. (yes, I did some test, only 1/3 programmers notice the inconsistency in the first place, though my sample is very small, only 15 person 😉) Even they know the inconsistency, most agree it's not a big deal.

[!-- Someone told me he always think a[b] syntax for property access is weird, it should be a.{b} (more like interpolation syntax), and as his understanding, a?.[prop] is much correct! And now he can treat a[b] as the shorthand of a.[b] 😂 --]

Keep ?. syntax means we can also keep ?? syntax for nullish coalescing, both consistent with many other languages. This is a very big win for easy to learning and adopting.

I hope this PR could get consensus in TC39 soon, we have waited these wonderful features too loooong!

dannyskoog commented 5 years ago

I agree with former speaker. ”Optional function execution” is something I personally would consider less prioritized. And if the journey of this proposal would easen up by leaving it outside of it, then that’s a no-brainer to me.

The square bracket syntax (”foo?.[bar]”) is obviously not ideal. But weighing it towards consistency, it’s definitely worth it.

hax commented 5 years ago

Glad to see this PR merged, does it mean TC39 has consensus on current syntax and semantic? Can we forward it to stage 3 in near future?

dusave commented 5 years ago

@hax We're not even at Stage 2 yet. I presented the updated proposal today and it seems like we're getting closer. A couple of notable concerns around consistency with the operator, but overall, I think most are agreeable to the proposal now. Will be seeking Stage 2 in March.

claudepache commented 5 years ago

@dustinsavery You merged the PR, but it had still an unaddressed technical issue:

You explicitly added ”optional deletion” in the ”Not supported” section of the README, but you didn’t in fact modify the normative text in order to remove ”optional deletion” support. Now, the README and the spec text contradict themselves.

As I said in https://github.com/tc39/proposal-optional-chaining/pull/73/files#r229990799, if you do want remove ”optional deletion” support, you must add explicit appropriate restrictions in the use of the delete operator.

dusave commented 5 years ago

@claudepache I spoke with @jridgewell about this and we agreed that as the spec currently stands, it doesn't need to be modified to actively reject optional chaining syntax. Could you clarify what needs to be updated?

claudepache commented 5 years ago

@dustinsavery I have already explained it in other comments, but I’ll repeat here with all details.

I guess that it may be confusing that:

Below are all the relevant technical details:

First, as the spec currently stands, delete a?.b is authorised by the lexical grammar; the relevant rules are, from the official spec:

UnaryExpression: delete UnaryExpression (per ecma262:prod-UnaryExpression) UnaryExpression: UpdateExpression (per ecma262:prod-UnaryExpression) UpdateExpression: LeftHandSideExpression (per ecma262:prod-UpdateExpression)

and from the proposal:

LeftHandSideExpression: OptionalExpression (per optional-chaining:prod-LeftHandSideExpression)

and a?.b is an OptionalExpression.

Concerning the runtime semantics of delete a?.b, quoting myself from https://github.com/tc39/proposal-optional-chaining/issues/40#issuecomment-345483067:

The runtime semantics of delete is: When it receives

  1. a non-Reference or an unresolvable Reference: do nothing (and pretend to have succeeded);
  2. a resolvable Reference: attempt to delete it (that would throw an ReferenceError in strict mode if the Reference is not deletable).

As currently specced, delete (null)?.x falls in case 1 ((null)?.x evaluates to undefined, not to a reference that resolves to undefined).

At this point, in order to be super-explicit, I must add: If a does not evaluates to null/undefined, delete a?.b falls in case 2. The conclusion remains:

The final outcome is exactly what a human would intuit.

If you want to remove optional deletion support. Quoting myself from https://github.com/tc39/proposal-optional-chaining/pull/73/files#r229990799:

If you want to remove ”optional deletion”, you must add additional static semantics in Section The delete Operator, in order to forbid specifically a LeftHandSideExpression containing an OptionalChain to be used with the delete operator.

My advice is: Unless you are willing to actively forbid optional deletion, note in the README that optional deletion is accidentally supported.

hax commented 5 years ago

So maybe we can just support delete x?.y if all behavior of it match programmers' intuition?

dusave commented 5 years ago

@claudepache I see where the problem was. Fortunately, it's now a moot point. We had a very productive breakout session today and I managed to get everyone on board with the original spec as was written prior to my modifications. So I have a new PR (#75) that will effectively revert all of my updates. Completely unexpected turn of events, but I'm happy none-the-less!