Class Private Static Accessors #10217

tim-mc · 2019-07-14T00:37:04Z

Q	A
Fixed Issues?	babel/proposals#56, closes #8052
Patch: Bug Fix?	--
Major: Breaking Change?	--
Minor: New Feature?	Add class private static accessor support
Tests Added + Pass?	Yes
Documentation PR Link
Any Dependency Changes?	No
License	MIT

cc @robpalme @nicolo-ribaudo @littledan

tim-mc · 2019-07-14T00:42:03Z

Still a WIP. Spec support has been added. Working on loose now.

babel-bot · 2019-07-14T00:48:23Z

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/11377/

tim-mc · 2019-07-15T15:52:37Z

Loose support has been added

packages/babel-helper-create-class-features-plugin/src/fields.js

JLHwung · 2019-07-18T14:02:59Z

packages/babel-helper-create-class-features-plugin/src/fields.js

@@ -300,8 +303,47 @@ function buildPrivateInstanceFieldInitSpec(ref, prop, privateNamesMap) {
 }

 function buildPrivateStaticFieldInitSpec(prop, privateNamesMap) {
-  const { id } = privateNamesMap.get(prop.node.key.id.name);
+  const privateName = privateNamesMap.get(prop.node.key.id.name);
+  const { id, getId, setId, initAdded } = privateName;
  const value = prop.node.value || prop.scope.buildUndefinedNode();


I suggest move this line immediately before where value is used so we can early return for non-accessor class private methods.

packages/babel-helper-create-class-features-plugin/src/fields.js

jridgewell · 2019-07-20T19:51:56Z

packages/babel-helper-create-class-features-plugin/src/fields.js

  const value = prop.node.value || prop.scope.buildUndefinedNode();
+  if (!prop.isProperty() && (initAdded || !(getId || setId))) return;


When would it not be a property?

It wouldn't be a property if it's a static private accessor.

Nit: could you extract getId || setId to an isAccessor boolean variable?
I find it hard to read all these nested boolean expressions.

jridgewell · 2019-07-20T19:52:51Z

packages/babel-helper-create-class-features-plugin/src/fields.js

+          // enumerable is false by default
+          // writable is false by default
+          get: ${getId.name},
+          set: ${setId.name}


Ditto reuse.

That works in this case, but setting null as a getter/setter here is a TypeError.

Ah, yah. We should be able to use undefined in all cases instead.

Good point, updated.

jridgewell · 2019-07-20T19:54:06Z

packages/babel-helper-create-class-features-plugin/src/fields.js

@@ -615,14 +697,14 @@ export function buildFieldsInitNodes(
      case isStatic && isPrivate && isMethod && !loose:
        needsClassRef = true;
        staticNodes.push(
+          buildPrivateStaticFieldInitSpec(prop, privateNamesMap),
+        );
+        staticNodes.unshift(


Why unshift?

I use unshift so that the declaration of the getter and/or setter occurs before the usage of the accessor(s) (e.g., when they're defined as the values on an object). Here's what a test failure looks like if that's changed to push:

babel/packages/babel-plugin-proposal-private-methods/test/fixtures/static-accessors/basic/exec.js: expect(received).toEqual(expected) // deep equality Expected: "top secret string" Received: undefined 1 | class Cl { 2 | static getValue() { 3 | return babelHelpers.classStaticPrivateFieldSpecGet(Cl, Cl, _privateStaticFieldValue); 4 | } 5 | 6 | static setValue() { 7 | babelHelpers.classStaticPrivateFieldSpecSet(Cl, Cl, _privateStaticFieldValue, "dank"); 8 | } 9 | 10 | } 11 | 12 | var _PRIVATE_STATIC_FIELD = { 13 | writable: true, 14 | value: "top secret string" 15 | }; 16 | var _privateStaticFieldValue = { 17 | get: _get_privateStaticFieldValue, 18 | set: _set_privateStaticFieldValue 19 | }; 20 | 21 | var _get_privateStaticFieldValue = function () { 22 | return babelHelpers.classStaticPrivateFieldSpecGet(Cl, Cl, _PRIVATE_STATIC_FIELD); 23 | }; 24 | 25 | var _set_privateStaticFieldValue = function (newValue) { 26 | babelHelpers.classStaticPrivateFieldSpecSet(Cl, Cl, _PRIVATE_STATIC_FIELD, `Updated: ${newValue}`); 27 | }; 28 | 29 | expect(Cl.getValue()).toEqual("top secret string"); 30 | Cl.setValue(); 31 | expect(Cl.getValue()).toEqual("Updated: dank");

Can we change the get/set variables to function declarations instead? If not, this is still fine.

packages/babel-helper-create-class-features-plugin/src/fields.js

nicolo-ribaudo · 2019-07-25T08:04:40Z

packages/babel-helper-create-class-features-plugin/src/fields.js

  const value = prop.node.value || prop.scope.buildUndefinedNode();
+  if (!prop.isProperty() && (initAdded || !(getId || setId))) return;


Nit: could you extract getId || setId to an isAccessor boolean variable?
I find it hard to read all these nested boolean expressions.

tim-mc force-pushed the private-static-accessors branch from 1de0bb8 to 07420b3 Compare July 14, 2019 00:39

tim-mc marked this pull request as ready for review July 15, 2019 15:52

nicolo-ribaudo self-requested a review July 15, 2019 16:02

tim-mc force-pushed the private-static-accessors branch 2 times, most recently from a8def51 to 60b2fa9 Compare July 17, 2019 23:44

JLHwung reviewed Jul 18, 2019

View reviewed changes

packages/babel-helper-create-class-features-plugin/src/fields.js Show resolved Hide resolved

JLHwung reviewed Jul 18, 2019

View reviewed changes

tim-mc force-pushed the private-static-accessors branch from 60b2fa9 to 14f480d Compare July 19, 2019 20:05

jridgewell approved these changes Jul 20, 2019

View reviewed changes

tim-mc force-pushed the private-static-accessors branch from 14f480d to ee9364e Compare July 22, 2019 13:48

jridgewell approved these changes Jul 22, 2019

View reviewed changes

packages/babel-helper-create-class-features-plugin/src/fields.js Outdated Show resolved Hide resolved

packages/babel-helper-create-class-features-plugin/src/fields.js Outdated Show resolved Hide resolved

nicolo-ribaudo approved these changes Jul 25, 2019

View reviewed changes

tim-mc force-pushed the private-static-accessors branch 2 times, most recently from 1bcccbd to 5ec8290 Compare July 29, 2019 15:51

nicolo-ribaudo added PR: New Feature 🚀 A type of pull request used for our changelog categories Spec: Private Methods labels Aug 1, 2019

nicolo-ribaudo added this to the v7.6.0 milestone Aug 1, 2019

nicolo-ribaudo added the PR: Ready to be Merged A pull request with already two approvals, but waiting for the next minor release label Aug 1, 2019

JLHwung approved these changes Aug 1, 2019

View reviewed changes

tim-mc added 8 commits August 23, 2019 14:07

Remove error for static private accessors

4584068

Private static accessors strict

d0e94da

Add loose mode support

231861b

Move value decl for early return

15ad1b5

Reuse getter/setter template

6f7ed38

Reuse getter/setter templates

3f8d021

Use buildUndefinedNode in accessor templates

3e6c955

Extract isAccessor variable

fc24d84

tim-mc force-pushed the private-static-accessors branch from 5ec8290 to fc24d84 Compare August 23, 2019 19:59

nicolo-ribaudo merged commit 3e4889d into babel:master Sep 6, 2019

This was referenced Sep 23, 2019

[Snyk] Upgrade @babel/runtime from 7.4.4 to 7.6.0 Matthelonianxl/drizzle#2

Open

[Snyk] Upgrade @babel/polyfill from 7.2.5 to 7.6.0 maxiplux/full-stack-fastapi-couchbase#5

Open

[Snyk] Upgrade @babel/runtime-corejs2 from 7.5.5 to 7.6.0 maxiplux/swagger-ui#5

Open

snyk-bot mentioned this pull request Oct 10, 2019

[Snyk] Upgrade @babel/runtime from 7.6.0 to 7.6.2 cds-snc/report-a-cybercrime#805

Merged

snyk-bot mentioned this pull request Oct 24, 2019

[Snyk] Upgrade @babel/runtime from 7.4.4 to 7.6.3 Matthelonianxl/drizzle#9

Open

snyk-bot mentioned this pull request Nov 1, 2019

[Snyk] Upgrade @babel/core from 7.2.2 to 7.6.4 nbv1982/material-ui#5

Open

snyk-bot mentioned this pull request Nov 12, 2019

[Snyk] Upgrade @babel/runtime from 7.1.5 to 7.6.3 ajesse11x/Parse-SDK-JS#1

Open

This was referenced Nov 28, 2019

[Snyk] Upgrade @babel/runtime from 7.0.0-beta.42 to 7.7.2 ali8889/mist#4

Open

[Snyk] Upgrade @babel/runtime from 7.5.5 to 7.7.4 dyna-dot/CyberChef#3

Open

snyk-bot mentioned this pull request Dec 5, 2019

[Snyk] Upgrade @babel/runtime from 7.3.1 to 7.7.4 dyna-dot/bit-javascript#5

Open

lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Dec 6, 2019

lock bot locked as resolved and limited conversation to collaborators Dec 6, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Class Private Static Accessors #10217

Class Private Static Accessors #10217

tim-mc commented Jul 14, 2019 •

edited by nicolo-ribaudo

tim-mc commented Jul 14, 2019

babel-bot commented Jul 14, 2019 •

edited

tim-mc commented Jul 15, 2019

JLHwung Jul 18, 2019

tim-mc Jul 22, 2019

jridgewell Jul 20, 2019

tim-mc Jul 22, 2019

nicolo-ribaudo Jul 25, 2019

jridgewell Jul 20, 2019

tim-mc Jul 22, 2019 •

edited

jridgewell Jul 22, 2019

tim-mc Jul 22, 2019

jridgewell Jul 20, 2019

tim-mc Jul 22, 2019

jridgewell Jul 22, 2019

nicolo-ribaudo Jul 25, 2019

		const value = prop.node.value \|\| prop.scope.buildUndefinedNode();
		if (!prop.isProperty() && (initAdded \|\| !(getId \|\| setId))) return;

Class Private Static Accessors #10217

Class Private Static Accessors #10217

Conversation

tim-mc commented Jul 14, 2019 • edited by nicolo-ribaudo

tim-mc commented Jul 14, 2019

babel-bot commented Jul 14, 2019 • edited

tim-mc commented Jul 15, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tim-mc Jul 22, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tim-mc commented Jul 14, 2019 •

edited by nicolo-ribaudo

babel-bot commented Jul 14, 2019 •

edited

tim-mc Jul 22, 2019 •

edited