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

@babel/plugin-proposal-class properties: `.name` incorrect for private fields #10175

Open
mheiber opened this issue Jul 7, 2019 · 7 comments

Comments

Projects
None yet
5 participants
@mheiber
Copy link

commented Jul 7, 2019

Bug Report

Current Behavior

When a function is assigned to a private field, its name property should (I think) be name of the private field. Instead, the name is always value.

Input Code

class A {
    #foo = function () {};
    constructor () {
        console.log(this.#foo.name);
    }
}

new A(); // should log something else (see below)

Expected behavior/code

The code above should log 'foo'.
The code above should log '#foo'

see comment below, I think I read the spec wrong before

Babel Configuration (.babelrc, package.json, cli command)

repl link

Possible Solution

The function generated WeakMap entry could use the name of the private field ('foo' in this case instead of 'value'). per the spec proposal

  _foo.set(this, {
    writable: true,
    value: function value() {}
  });

Side note: not sure what the 'writable' part is doing: what's a read-only private field?

Additional context/Screenshots

@babel-bot

This comment has been minimized.

Copy link
Collaborator

commented Jul 7, 2019

Hey @mheiber! We really appreciate you taking the time to report an issue. The collaborators
on this project attempt to help as many people as possible, but we're a limited number of volunteers,
so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack
community
that typically always has someone willing to help. You can sign-up here
for an invite.

@JLHwung

This comment has been minimized.

Copy link
Contributor

commented Jul 12, 2019

It seems that V8 will print #foo as the function name instead of foo.

class A {
    #foo = function () {};
    constructor () {
        console.log(this.#foo.name, this.#foo.name.length);
    }
}

new A();

prints #foo 4 instead of foo 3 according to the spec.

I have submitted a bug report to V8: https://bugs.chromium.org/p/v8/issues/detail?id=9481

@mheiber

This comment has been minimized.

Copy link
Author

commented Jul 12, 2019

Hi @JLHwung whoops! I think I made a mistake earlier. Sorry about that.

It looks to me now that the function.name should be #foo:

https://tc39.es/proposal-class-fields/#sec-runtime-semantics-evaluate-name

lexical semantics:

  • privateIdentifier

runtime semantics
names are created with newPrivateName(privateIdentifier)

in the definition of newPrivateName(description):
- [[description]] is set to description

in the lexical grammar:

  • privateIdentifier :: # identifierName
@mheiber

This comment has been minimized.

Copy link
Author

commented Jul 17, 2019

Does this seem to be a real issue, then? I could make a PR in a few weeks after I get approval from my company to contribute.

@JLHwung

This comment has been minimized.

Copy link
Contributor

commented Jul 17, 2019

@mheiber It is a bug and PR is definitely appreciated.

Note that Babel has similar issues on keeping track of .name property, (i.e. #7425 and #9562). Most of this issue can be solved by guess a name from AST and use it as the BindingIdentifier name of the function declaration.

However, as # is not a valid character for identifier name. This one will be more difficult to tackle with than those I mention above. We may have to append Object.defineProperty calls to the function declaration.

@nicolo-ribaudo

This comment has been minimized.

Copy link
Member

commented Jul 17, 2019

I would prefer not to use defineProperty and to give the name without the hash. Function names are one of the parts of the spec where we aren't really compliant, since what matters most is that they are usable for debugging.

@JLHwung

This comment has been minimized.

Copy link
Contributor

commented Jul 18, 2019

Function names are one of the parts of the spec where we aren't really compliant, since what matters most is that they are usable for debugging.

I agree, at least it will be better than our current situation, though I am afraid it might confuse developers if we return the name without hash while native implementations return the name with hash, which is spec compliant. Maybe we can leave a note comment on that.

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.