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

destructuring private fields with array pattern / object pattern #10017

13 changes: 13 additions & 0 deletions packages/babel-helper-create-class-features-plugin/src/fields.js
Expand Up @@ -211,6 +211,19 @@ const privateNameHandlerSpec = {
]);
},

destructureSet(member) {
const { privateNamesMap, file } = this;
const { name } = member.node.property.id;
const { id } = privateNamesMap.get(name);
return t.memberExpression(
t.callExpression(file.addHelper("classPrivateFieldDestructureSet"), [
this.receiver(member),
t.cloneNode(id),
]),
t.identifier("value"),
);
},

call(member, args) {
// The first access (the get) should do the memo assignment.
this.memoise(member, 1);
Expand Down
28 changes: 28 additions & 0 deletions packages/babel-helper-member-expression-to-functions/src/index.js
Expand Up @@ -100,6 +100,34 @@ const handle = {
return;
}

// { KEY: MEMBER } = OBJ -> { KEY: _destructureSet(MEMBER) } = OBJ
// { KEY: MEMBER = _VALUE } = OBJ -> { KEY: _destructureSet(MEMBER) = _VALUE } = OBJ
// {...MEMBER} -> {..._destructureSet(MEMBER)}
//
// [MEMBER] = ARR -> [_destructureSet(MEMBER)] = ARR
// [MEMBER = _VALUE] = ARR -> [_destructureSet(MEMBER) = _VALUE] = ARR
// [...MEMBER] -> [..._destructureSet(MEMBER)]
if (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I remember correctly, this will affect super.foo expressions, too. Can you add a test for assigning [super.foo] = array, too?

// { KEY: MEMBER } = OBJ
(parentPath.isObjectProperty({ value: node }) &&
parentPath.parentPath.isObjectPattern()) ||
// { KEY: MEMBER = _VALUE } = OBJ
(parentPath.isAssignmentPattern({ left: node }) &&
parentPath.parentPath.isObjectProperty({ value: parent }) &&
parentPath.parentPath.parentPath.isObjectPattern()) ||
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I think we can actually join a few of the conditions here. parentPath.isAssignmentPattern() is sufficient to check for both array pattern and object pattern, we don't need to consult the grand parent.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i guess so, it's just to make my intentions clearer, not sure will assignment pattern be a child of not ust ObjectPattern and ArrayPattern ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can move these checks to be assertions: (t.assertObjectProperty(parentPath.parentPath, { value: parent })

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nicolo-ribaudo what is the rationale behind using assertions instead of is?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just that with an assertion is clear that if this is an assignment expression, the parent will always be an object/array destructing

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh ok nevermind, we can't assert two different parent types.

// [MEMBER] = ARR
parentPath.isArrayPattern() ||
// [MEMBER = _VALUE] = ARR
(parentPath.isAssignmentPattern({ left: node }) &&
parentPath.parentPath.isArrayPattern()) ||
// {...MEMBER}
// [...MEMBER]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Man, some of these syntaxes are crazy.

parentPath.isRestElement()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we also need to check for AssignmentPatterns?

[this.#foo = 2] = []

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

checked and added more test case

) {
member.replaceWith(this.destructureSet(member));
return;
}

// MEMBER -> _get(MEMBER)
member.replaceWith(this.get(member));
},
Expand Down
13 changes: 13 additions & 0 deletions packages/babel-helper-replace-supers/src/index.js
Expand Up @@ -125,6 +125,12 @@ const specHandlers = {
]);
},

destructureSet(superMember) {
throw superMember.buildCodeFrameError(
`Destructuring to a super field is not supported yet.`,
);
},

call(superMember, args) {
return optimiseCall(this.get(superMember), t.thisExpression(), args);
},
Expand Down Expand Up @@ -174,6 +180,13 @@ const looseHandlers = {
value,
);
},

destructureSet(superMember) {
const { computed } = superMember.node;
const prop = this.prop(superMember);

return t.memberExpression(t.thisExpression(), prop, computed);
},
};

export default class ReplaceSupers {
Expand Down
30 changes: 28 additions & 2 deletions packages/babel-helpers/src/helpers.js
Expand Up @@ -701,7 +701,6 @@ helpers.superPropBase = helper("7.0.0-beta.0")`
`;

helpers.get = helper("7.0.0-beta.0")`
import getPrototypeOf from "getPrototypeOf";
import superPropBase from "superPropBase";

export default function _get(target, property, receiver) {
Expand All @@ -726,7 +725,6 @@ helpers.get = helper("7.0.0-beta.0")`
`;

helpers.set = helper("7.0.0-beta.0")`
import getPrototypeOf from "getPrototypeOf";
import superPropBase from "superPropBase";
import defineProperty from "defineProperty";

Expand Down Expand Up @@ -1104,6 +1102,34 @@ helpers.classPrivateFieldSet = helper("7.0.0-beta.0")`
}
`;

helpers.classPrivateFieldDestructureSet = helper("7.4.4")`
export default function _classPrivateFieldDestructureSet(receiver, privateMap) {
if (!privateMap.has(receiver)) {
throw new TypeError("attempted to set private field on non-instance");
}
var descriptor = privateMap.get(receiver);
if (descriptor.set) {
if (!("__destrObj" in descriptor)) {
descriptor.__destrObj = {
set value(v) {
descriptor.set.call(receiver, v)
},
};
}
return descriptor.__destrObj;
} else {
if (!descriptor.writable) {
// This should only throw in strict mode, but class bodies are
// always strict and private fields can only be used inside
// class bodies.
throw new TypeError("attempted to set read only private field");
}

return descriptor;
}
}
`;

helpers.classStaticPrivateFieldSpecGet = helper("7.0.2")`
export default function _classStaticPrivateFieldSpecGet(receiver, classConstructor, descriptor) {
if (receiver !== classConstructor) {
Expand Down
@@ -0,0 +1,17 @@
class Foo {
#client

constructor(props) {
this.#client = 1;
;([this.x = this.#client, this.#client, this.y = this.#client] = props);
}

getClient() {
return this.#client;
}
}

const foo = new Foo([undefined, 'bar']);
expect(foo.getClient()).toBe('bar');
expect(foo.x).toBe(1);
expect(foo.y).toBe('bar');
@@ -0,0 +1,8 @@
class Foo {
#client

constructor(props) {
this.#client = 1;
([this.x = this.#client, this.#client, this.y = this.#client] = props);
}
}
@@ -0,0 +1,13 @@
var Foo = function Foo(props) {
"use strict";

babelHelpers.classCallCheck(this, Foo);
Object.defineProperty(this, _client, {
writable: true,
value: void 0
});
babelHelpers.classPrivateFieldLooseBase(this, _client)[_client] = 1;
[this.x = babelHelpers.classPrivateFieldLooseBase(this, _client)[_client], babelHelpers.classPrivateFieldLooseBase(this, _client)[_client], this.y = babelHelpers.classPrivateFieldLooseBase(this, _client)[_client]] = props;
};

var _client = babelHelpers.classPrivateFieldLooseKey("client");
@@ -0,0 +1,15 @@
class Foo {
#client

constructor(props) {
let x;
;([x, ...this.#client] = props);
}

getClient() {
return this.#client;
}
}

const foo = new Foo(['foo', 'bar', 'baz', 'quu']);
expect(foo.getClient()).toEqual(['bar', 'baz', 'quu']);
@@ -0,0 +1,7 @@
class Foo {
#client

constructor(props) {
([x, ...this.#client] = props);
}
}
@@ -0,0 +1,12 @@
var Foo = function Foo(props) {
"use strict";

babelHelpers.classCallCheck(this, Foo);
Object.defineProperty(this, _client, {
writable: true,
value: void 0
});
[x, ...babelHelpers.classPrivateFieldLooseBase(this, _client)[_client]] = props;
};

var _client = babelHelpers.classPrivateFieldLooseKey("client");
@@ -0,0 +1,14 @@
class Foo {
#client

constructor(props) {
([this.#client = 5] = props);
}

getClient() {
return this.#client;
}
}

const foo = new Foo([]);
expect(foo.getClient()).toEqual(5);
@@ -0,0 +1,7 @@
class Foo {
#client

constructor(props) {
([this.#client = 5] = props);
}
}
@@ -0,0 +1,12 @@
var Foo = function Foo(props) {
"use strict";

babelHelpers.classCallCheck(this, Foo);
Object.defineProperty(this, _client, {
writable: true,
value: void 0
});
[babelHelpers.classPrivateFieldLooseBase(this, _client)[_client] = 5] = props;
};

var _client = babelHelpers.classPrivateFieldLooseKey("client");
@@ -0,0 +1,14 @@
class Foo {
#client

constructor(props) {
;([this.#client] = props);
}

getClient() {
return this.#client;
}
}

const foo = new Foo(['bar']);
expect(foo.getClient()).toBe('bar');
@@ -0,0 +1,7 @@
class Foo {
#client

constructor(props) {
([this.#client] = props);
}
}
@@ -0,0 +1,12 @@
var Foo = function Foo(props) {
"use strict";

babelHelpers.classCallCheck(this, Foo);
Object.defineProperty(this, _client, {
writable: true,
value: void 0
});
[babelHelpers.classPrivateFieldLooseBase(this, _client)[_client]] = props;
};

var _client = babelHelpers.classPrivateFieldLooseKey("client");
@@ -0,0 +1,17 @@
class Foo {
#client

constructor(props) {
this.#client = 'foo';
;({ x: this.x = this.#client, y: this.#client, z: this.z = this.#client } = props)
}

getClient() {
return this.#client;
}
}

const foo = new Foo({ y: 'bar' });
expect(foo.getClient()).toBe('bar');
expect(foo.x).toBe('foo');
expect(foo.z).toBe('bar');
@@ -0,0 +1,8 @@
class Foo {
#client

constructor(props) {
this.#client = 'foo';
({ x: this.x = this.#client, y: this.#client, z: this.z = this.#client } = props)
}
}
@@ -0,0 +1,17 @@
var Foo = function Foo(props) {
"use strict";

babelHelpers.classCallCheck(this, Foo);
Object.defineProperty(this, _client, {
writable: true,
value: void 0
});
babelHelpers.classPrivateFieldLooseBase(this, _client)[_client] = 'foo';
({
x: this.x = babelHelpers.classPrivateFieldLooseBase(this, _client)[_client],
y: babelHelpers.classPrivateFieldLooseBase(this, _client)[_client],
z: this.z = babelHelpers.classPrivateFieldLooseBase(this, _client)[_client]
} = props);
};

var _client = babelHelpers.classPrivateFieldLooseKey("client");
@@ -0,0 +1,15 @@
class Foo {
#client

constructor(props) {
let x;
;({ x, ...this.#client } = props)
}

getClient() {
return this.#client;
}
}

const foo = new Foo({ x: 'foo', y: 'bar', z: 'baz' });
expect(foo.getClient()).toEqual({ y: 'bar', z: 'baz' });
@@ -0,0 +1,15 @@
{
"plugins": [
"transform-destructuring",
[
"external-helpers",
{
"helperVersion": "7.4.4"
}
],
"proposal-class-properties",
"transform-classes",
"transform-block-scoping",
"syntax-class-properties"
]
}
@@ -0,0 +1,7 @@
class Foo {
#client

constructor(props) {
({ x, ...this.#client } = props)
}
}
@@ -0,0 +1,15 @@
var Foo = function Foo(props) {
"use strict";

babelHelpers.classCallCheck(this, Foo);
Object.defineProperty(this, _client, {
writable: true,
value: void 0
});
({
x,
...babelHelpers.classPrivateFieldLooseBase(this, _client)[_client]
} = props);
};

var _client = babelHelpers.classPrivateFieldLooseKey("client");