Closed mbrowne closed 5 years ago
The shortened syntax shouldn’t exist imo - since yo need to be able to access it on any arbitrary object, requiring an explicit indicator that you’re accessing it on this
(another arbitrary object) is a good consistency.
The thing is, this is not a shorthand syntax. Take a look at the /docs/definitions.md file. The instance and class closures are execution contexts that are added to the function scope chain just as if the call to the current function was made from within that execution context. That means that all let
and const
definitions are just naturally available. There's nothing extra the engine needs to do to resolve these references beyond the normal process.
Now, feel free to convince me this isn't a good thing. If that works out to be the case, the notation reverts to obj::variable
.
Having a difference between how you access the private instance fields/properties/variables on the receiver versus on other objects is a problem - it makes the concepts harder to understand and teach, and allowing both a lexical identifier foo
and this::foo
is fodder for developer bikeshedding and an increased burden on linters to enforce one form or the other.
Individually, those arguments don't really hold.
That being said, together those problems sound like an annoying combination. I'll leave myself flexible on this issue. I'd really want more opinions or maybe a test run to see if having lexical identifiers really does pose any issues. However, if TC39 couldn't swallow this proposal over this small issue, I could live without it.
@ljharb This doesn't seem to be an issue for other languages, just a nice convenience. And there isn't much bikeshedding either, but rather a general consensus to use just the property name rather than this.propertyName
or self.propertyName
, unless there's some shadowing going on that makes it necessary to be more explicit.
By bikeshedding i mean, each project has to decide that for itself. We should strive to provide only one way to do things.
There’s a reason shorthand syntax was dropped entirely from the current proposal. I’d suggest doing the same.
I was under the impression that shorthand syntax was dropped from the current proposal because it posed insurmountable issues, not the least of which was a collision problem between #x
as a definition and #x
as an access. #x = 2
in a class that didn't define #x
looked like it should dynamically add the field, which was a no-no.
There's no such problems for the syntax of this proposal. There's always a clear distinction between a definition and an access. Allowing lexical name access is much more ergonomic as well. However, if it's the consensus of TC39 that this kind of access shouldn't be allowed, that's an acceptable loss.
It’d also that there was an asymmetry between access on the receiver and other objects - which your proposal doesn’t resolve.
Now that's incontrovertibly true. The only precedent for this in the language is with
, which has been given a really bad rep. I really like the ergonomics of it..... but I just had a thought that killed it for me. While instance variables don't need any object reference to access them, properties do, but their both "members" of that object. That inconsistency is too distasteful for me.
2nd win for Jordan Harband!!!
I'll remove that capability from the proposal.
Given that lexical name access is no longer a part of this, it seems like it would be safe to downgrade instance and class closures from an execution context to a (special case? of a) function environment record. Does that make sense? This will likely make this proposal easier to implement.
Can you elaborate on what the difference would be?
It's a matter of the implementation. I just finished reading more of the spec. I think I've got the picture straight now, but I'd still like your opinion.
shared
members would be the chain of environment records for the lexical environment of that function.In this way, a statement that looks like this:
this::foo;
would be a call to GetBindingValue
against the attached environment record, and
this::foo = "bar";
would be a call to SetMutableBinding
.
Lexical name access has been removed.
I looked in the spec but couldn't find any mention of this feature mentioned in the readme:
Maybe I missed it, or maybe you just didn't update the spec to include the shorthand syntax yet (I see that https://github.com/rdking/proposal-class-members/blob/master/src/runtime-semantics.html still refers to
->
syntax so I assume you're still in the process of updating things.)Separately, I wanted to point out that the github website field for this repo is still pointed here: https://zenparsing.github.io/js-classes-1.1/
While this proposal is still evolving, perhaps it would be better/easier to remove the website field, and then when things stabilize a bit you could publish your own web page with the updated spec for this proposal.
Having said all that, I want to also say that I really respect the amount of work you have put into creating a completely specified alternative proposal (and of course, all the work that went into creating classes 1.1). I think this can help us all be more clear on the tradeoffs when comparing this with the class-fields proposal.