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

Name property of anonymous class extending another classes is set inconsistently #36038

Closed
StenCalDabran opened this issue Nov 8, 2020 · 4 comments
Labels
v8 engine Issues and PRs related to the V8 dependency.

Comments

@StenCalDabran
Copy link

Setup

  • Version: one behaviour for latest 12 (12.19.0) and 15 versions (15.0.1), the other behaviour for the latest 14 (14.15.0) and older 12 versions (e.g. 12.13.0)
  • Platform: Platform-independent (same behaviour on Windows, Mac and within Docker container)
  • Subsystem: ?

What steps will reproduce the bug?

Run the following Javascript code

class SomeClass {}
console.log('Anonymous name property is:', class extends SomeClass {}.name);

How often does it reproduce? Is there a required condition?

No additional conditions beside node version.

What is the expected behavior?

I am not sure what the expected behaviour is, but it should be consistent at least for all latest minor versions of still supported major versions

What do you see instead?

For version 12.19.0 and 15.0.1 the name property of the anonymous class is an empty string.
For version 12.13.0 and 14.15.0 the name property of the anonymous class is the name of the extended class, i.e. 'SomeClass' in the example.

@devsnek
Copy link
Member

devsnek commented Nov 8, 2020

JS was changed recently to always set the name property on anonymous functions/classes. Prior to this, you were just seeing the name property from the class it extends due to prototypal inheritance.

@StenCalDabran
Copy link
Author

Ok, so if the expected behaviour now is that the name property of the extended class always gets overwritten, then this should also happen in the latest stable version? It seems weird that this change is already implemented in the latest 12 version but not yet in the latest 14 version.

@aduh95
Copy link
Contributor

aduh95 commented Dec 12, 2020

The spec change happened in tc39/ecma262@74a9ef8, and it's been implemented in V8 7.8 (v8/v8@48c9ca4) which is why Node.js v12.16+ shows the correct behaviour. However it's been reverted by v8/v8@1b594a2 that landed in V8 8.0 (Node.js v14 uses V8 8.4). It's since been re-implemented by v8/v8@048761a, which landed in V8 8.6 / Node.js v15.0.0.
I've found the info in the V8:9646 bug ticket.

@StenCalDabran If you'd like to fix the behaviour of Node.js v14.x, you can open a backport PR to cherry-pick v8/v8@048761a onto v14.x-staging.

StenCalDabran added a commit to StenCalDabran/node that referenced this issue Dec 15, 2020
Original commit message:

    Install "name" property on anonymous classes

    This is a normative PR that reached consensus at the June 2019 TC39:
    tc39/test262#2299

    Bug: v8:9646
    Change-Id: I8cb927b9e9231dfb71ebf47171205a096350e38b
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2360905
    Reviewed-by: Marja Hölttä <marja@chromium.org>
    Commit-Queue: Shu-yu Guo <syg@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#69460}

Refs:  v8/v8@048761a

Closes: nodejs#36038
@targos targos added v14.x v8 engine Issues and PRs related to the V8 dependency. labels Dec 27, 2020
targos pushed a commit to StenCalDabran/node that referenced this issue Apr 24, 2021
Original commit message:

    Install "name" property on anonymous classes

    This is a normative PR that reached consensus at the June 2019 TC39:
    tc39/test262#2299

    Bug: v8:9646
    Change-Id: I8cb927b9e9231dfb71ebf47171205a096350e38b
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2360905
    Reviewed-by: Marja Hölttä <marja@chromium.org>
    Commit-Queue: Shu-yu Guo <syg@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#69460}

Refs: v8/v8@048761a

Closes: nodejs#36038
@targos
Copy link
Member

targos commented Nov 20, 2021

v14.x is now in Maintenance. Only critical bug fixes can land on it.

@targos targos closed this as completed Nov 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

No branches or pull requests

4 participants