Normative: Remove PrivateName constructor from decorators#127
Normative: Remove PrivateName constructor from decorators#127
Conversation
The constructor can be polyfilled in 5 lines of code; there may be a cleaner way to include it in the standard library in the future, or it may be made unnecessary in favor of certain future syntax. Therefore, it is not necessary to pass it as the second argument for all decorators. This patch also updates the code samples in the documentation (some of which was a bit out of date).
jridgewell
left a comment
There was a problem hiding this comment.
Please include this in the presentation slides. It feels weird, but I'm not opposed since a native PrivateName instance can be extracted without much hassle.
| this.#names.set(string, key); | ||
| return descriptor; | ||
| expose = descriptor => { | ||
| let key = descriptor.key; |
There was a problem hiding this comment.
is it impossible here for the key to be null or undefined?
There was a problem hiding this comment.
The key will always be either a String, a Symbol, or a PrivateName.
friend.js
Outdated
| expose = descriptor => { | ||
| let key = descriptor.key; | ||
| let string = key.toString(); | ||
| if (typeof key !== "privatename") { |
There was a problem hiding this comment.
is privatename still a new primitive? I thought it was a normal object now
There was a problem hiding this comment.
Ah, good catch, it's definitely an object.
|
@jridgewell Yes, I'll add a slide about this change (and #124) |
|
Given the positive response so far, I'll land this PR, but I'm still open to reasons to reconsider. |
|
Btw, this will make the Babel implementation of PrivateName easier, since I had to implement a "public constructor" and an "internal constructor" |
The constructor can be polyfilled in 5 lines of code; there may
be a cleaner way to include it in the standard library in the
future, or it may be made unnecessary in favor of certain future
syntax. Therefore, it is not necessary to pass it as the second
argument for all decorators.
This patch also updates the code samples in the documentation
(some of which was a bit out of date).
This PR addresses some comments by @caridy in #124.