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

Updating spec text #73

Merged
merged 4 commits into from Nov 29, 2018

Conversation

Projects
None yet
@dustinsavery
Copy link
Member

commented Oct 30, 2018

This update attempts to simplify this proposal. It:

  • #59 Removes optional function execution
    - #69 Changes the return from automatic undefined to null/undefined based on value.
@Mouvedia

This comment has been minimized.

Copy link

commented Oct 30, 2018

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

Could you link the issue related to this change?

@@ -134,37 +117,22 @@ That follows from the design choice of specifying the scope of short-circuiting

This comment has been minimized.

Copy link
@jridgewell

jridgewell Oct 30, 2018

Member

Above desugaring is incorrect.

@@ -134,37 +117,22 @@ That follows from the design choice of specifying the scope of short-circuiting
Note that, whatever the semantics are, there is no practical reason to use parentheses in that position anyway.
### Optional deletion

This comment has been minimized.

Copy link
@jridgewell

This comment has been minimized.

Copy link
@claudepache

claudepache Nov 1, 2018

Collaborator

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.

As I said in #40, ”Optional deletion has very few practical use cases; but forbidding it has probably no practical utility either.” However, forbidding optional deletion adds positive complexity to the spec.

* optional function execution: `a?.()`

This comment has been minimized.

Copy link
@jridgewell

jridgewell Oct 30, 2018

Member

👏#59

README.md Outdated
The following is not supported, although it has some use cases; see [Issue #18](//github.com/tc39/proposal-optional-chaining/issues/18) for discussion:
* optional property assignment: `a?.b = c`
* optional assignment: `a?.b?.c = x`

This comment has been minimized.

Copy link
@jridgewell

jridgewell Oct 30, 2018

Member

Nit: a?.b = x was simpler

This comment has been minimized.

Copy link
@claudepache

claudepache Nov 1, 2018

Collaborator

Also, ”optional property assignment” was more precise than ”optional assignment”. We’re not discussing about an optional version of a = b, but about an optional version of a.b = c.

@@ -590,12 +535,6 @@ <h1>Runtime Semantics: ChainEvaluation</h1>
1. If the code matched by this production is strict mode code, let _strict_ be *true*, else let _strict_ be *false*.
1. Return ? EvaluateStaticPropertyAccess(_baseValue_, |IdentifierName|, _strict_).
</emu-alg>
<emu-grammar>OptionalChain : OptionalChainingPunctuator Arguments</emu-grammar>

This comment has been minimized.

Copy link
@jridgewell

jridgewell Oct 30, 2018

Member

We need to delete the OptionalChainingPunctuator Arguments [?Yield, ?Await] in the OptionalChain [Yield, Await] syntax.

spec.html Outdated
@@ -612,15 +551,6 @@ <h1>Runtime Semantics: ChainEvaluation</h1>
1. If the code matched by this production is strict mode code, let _strict_ be *true*, else let _strict_ be *false*.
1. Return ? EvaluateStaticPropertyAccess(_newValue_, |IdentifierName|, _strict_).
</emu-alg>
<emu-grammar>OptionalChain : OptionalChain Arguments</emu-grammar>

This comment has been minimized.

Copy link
@jridgewell

jridgewell Oct 30, 2018

Member

This is still necessary, it defines the runtime semantics for a?.b()

spec.html Outdated
@@ -651,7 +581,6 @@ <h1>Expression Rules</h1>
<emu-grammar>
OptionalExpression :
MemberExpression OptionalChain
CallExpression OptionalChain

This comment has been minimized.

Copy link
@jridgewell

jridgewell Oct 30, 2018

Member

This defines tail call semantics for return a()?.b. You're looking for the OptionalChainingPunctuator Arguments below.

@jridgewell

This comment has been minimized.

Copy link
Member

commented Oct 30, 2018

Thanks for making a PR!

@littledan

This comment has been minimized.

Copy link
Member

commented Oct 30, 2018

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

@zenparsing

This comment has been minimized.

Copy link
Member

commented Oct 30, 2018

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 #69 (comment)).

@MatthiasKunnen

This comment has been minimized.

Copy link

commented Oct 31, 2018

@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 #69 (comment).

@Zarel

This comment has been minimized.

Copy link

commented Oct 31, 2018

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

This comment has been minimized.

Copy link

commented Oct 31, 2018

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:

  • Angular uses "always null" because its feature was ported from Dart, which doesn't distinguish between null and undefined. (source)
@ljharb

This comment has been minimized.

Copy link
Member

commented Oct 31, 2018

@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

This comment has been minimized.

Copy link

commented Oct 31, 2018

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

@Mouvedia

This comment has been minimized.

Copy link

commented Oct 31, 2018

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

  • lodash.get: 2 471 846 weekly downloads
  • idx: 69728 weekly downloads

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

This comment has been minimized.

Copy link
Member

commented Oct 31, 2018

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.

@dustinsavery

This comment has been minimized.

Copy link
Member Author

commented Oct 31, 2018

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:

  • I wanted to be careful when using "well, these languages do it, so we should too" as a foundational argument. After all, languages and libraries often implement things differently, each with their own reasons. That isn't to say that their reasons don't align with ours, just that I wanted to test our own reasoning before jumping on board.
  • This particular argument around providing context made sense to me, however upon further reflection and reading the comments above, I'm actually leaning towards always undefined as the better resolution, especially when considering how it impacts function default parameters.

Thanks to everyone for voicing their opinions on the matter 😄

@ljharb

This comment has been minimized.

Copy link
Member

commented Oct 31, 2018

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

This comment has been minimized.

Copy link
Member

commented Oct 31, 2018

@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

This comment has been minimized.

Copy link

commented Oct 31, 2018

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

This comment has been minimized.

Copy link
Member

commented Oct 31, 2018

@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. ?.[)?

@jridgewell
Copy link
Member

left a comment

There's still a OptionalChainingPunctuator Arguments in the Tail Position Calls section that needs to be removed.

spec.html Outdated
<emu-clause id="sec-optional-chaining-evaluation">
<h1>Runtime Semantics: Evaluation</h1>
<emu-grammar>
OptionalExpression :
MemberExpression OptionalChain
CallExpression OptionalChain

This comment has been minimized.

Copy link
@jridgewell

jridgewell Oct 31, 2018

Member

This one's still needed.

This comment has been minimized.

Copy link
@dustinsavery

dustinsavery Oct 31, 2018

Author Member

Thought I got that one, fixing now.

@dustinsavery

This comment has been minimized.

Copy link
Member Author

commented Oct 31, 2018

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

This comment has been minimized.

Copy link

commented Oct 31, 2018

@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

@dustinsavery

This comment has been minimized.

Copy link
Member Author

commented Oct 31, 2018

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

@ljharb

This comment has been minimized.

Copy link
Member

commented Oct 31, 2018

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

@Zarel

This comment has been minimized.

Copy link

commented Oct 31, 2018

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

#48 (comment)

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

@dustinsavery

This comment has been minimized.

Copy link
Member Author

commented Oct 31, 2018

@Zarel

The committee rejected dropping ?.[] in March

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

@littledan

This comment has been minimized.

Copy link
Member

commented Oct 31, 2018

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

This comment has been minimized.

Copy link
Member

commented Oct 31, 2018

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)

  • optional bracket access is imperative, because of Symbols as well as dynamic access (ie, "just member access" would be unacceptable)
  • grammar conflicts must be avoided, which is why ?[ and ?( are not available
  • symmetry between the tokens is imperative, ie, <something>., <something>[, <something>(
  • symmetry/similarity between optional chaining, and nullish coalescing, is important (some believe that it'd be fine if a reasonable operator for nullish coalescing was sufficiently different from the optional chaining operators)
  • optional call is problematic and should be dropped
  • the <something> mentioned above must have a connotation of "optional", which is why ? works particularly well, and why many of the options in #51 weren't deemed workable
  • null vs undefined input:
    • "null and undefined must both work as input"
    • "only undefined should work as input"
  • null vs undefined output:
    • "(null)?.a should produce null, (undefined)?.a should produce undefined"
    • "(null)?.a should produce undefined, (undefined)?.a should produce undefined"

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

This comment has been minimized.

Copy link

commented Oct 31, 2018

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

@jridgewell

This comment has been minimized.

Copy link
Member

commented Nov 1, 2018

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

This comment has been minimized.

Copy link
Member

commented Nov 1, 2018

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

@noppa

This comment has been minimized.

Copy link

commented Nov 1, 2018

@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?

spec.html Outdated
@@ -558,7 +502,6 @@ <h1>Tagged Templates (<a href="https://tc39.github.io/ecma262/#sec-tagged-templa
<emu-clause id="sec-optional-chains">
<h1>Optional Chains</h1>
<emu-note>An optional chain is a chain of property accesses and function calls introduced by an |OptionalChainingPunctuator|.</emu-note>

This comment has been minimized.

Copy link
@claudepache

claudepache Nov 1, 2018

Collaborator

If you remove ”optional function call”, this description is no more exact, and should be somewhat complexified, e.g.:

An optional chain is a chain introduced by an |OptionalChainingPunctuator|, and consisting of a property access facultatively followed by other property accesses and/or function calls.

(IOW, removing optional function calls does mean removing some form of regularity; so that I contend that it does not really ”simplify this proposal”.)

@dustinsavery

This comment has been minimized.

Copy link
Member Author

commented Nov 1, 2018

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

This comment has been minimized.

Copy link
Member

commented Nov 16, 2018

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

This comment has been minimized.

Copy link

commented Nov 16, 2018

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.

@dustinsavery dustinsavery merged commit 07d7f98 into master Nov 29, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@dustinsavery dustinsavery deleted the spec-update branch Nov 29, 2018

@hax

This comment has been minimized.

Copy link
Member

commented Nov 29, 2018

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?

@dustinsavery

This comment has been minimized.

Copy link
Member Author

commented Nov 29, 2018

@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

This comment has been minimized.

Copy link
Collaborator

commented Nov 29, 2018

@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.

@dustinsavery

This comment has been minimized.

Copy link
Member Author

commented Nov 29, 2018

@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

This comment has been minimized.

Copy link
Collaborator

commented Nov 29, 2018

@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:

  • if you do nothing you do support optional deletion;
  • if you want not to support optional deletion, explicit additional text is needed.

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 #40 (comment):

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

This comment has been minimized.

Copy link
Member

commented Nov 29, 2018

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

@dustinsavery

This comment has been minimized.

Copy link
Member Author

commented Nov 29, 2018

@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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.