Skip to content

Commit

Permalink
Private Class Methods Stage 3: Private Accessors (#9101)
Browse files Browse the repository at this point in the history 
* Add accessor loose support

* Add private accessors spec support

* Fix private dupe name check

* Changes from code review

* Add duplicated names tests

* Add get/set-only tests

* Move accessors tests

* Split out updates tests

* Add helper change tests

* Update test output

* Update test options
  • Loading branch information
@tim-mc @nicolo-ribaudo
tim-mc authored and nicolo-ribaudo committed Jan 21, 2019
1 parent 3ae5e79 commit e8de6fa
Show file tree
Hide file tree
Showing 53 changed files with 1,170 additions and 56 deletions.
6 changes: 0 additions & 6 deletions packages/babel-helper-create-class-features-plugin/src/features.js
View file
Expand Up @@ -72,12 +72,6 @@ export function verifyUsedFeatures(path, file) {
"@babel/plugin-class-features doesn't support class static private methods yet.",
);
}

if (path.node.kind !== "method") {
throw path.buildCodeFrameError(
"@babel/plugin-class-features doesn't support class private accessors yet.",
);
}
}

if (
Expand Down
206 changes: 168 additions & 38 deletions packages/babel-helper-create-class-features-plugin/src/fields.js
View file
Expand Up @@ -6,16 +6,26 @@ import optimiseCall from "@babel/helper-optimise-call-expression";
export function buildPrivateNamesMap(props) {
const privateNamesMap = new Map();
for (const prop of props) {
if (prop.isPrivate()) {
const isPrivate = prop.isPrivate();
const isMethod = !prop.isProperty();
const isInstance = !prop.node.static;
if (isPrivate) {
const { name } = prop.node.key.id;
privateNamesMap.set(name, {
id: prop.scope.generateUidIdentifier(name),
static: !!prop.node.static,
method: prop.isMethod(),
methodId: prop.isMethod()
? prop.scope.generateUidIdentifier(name)
: undefined,
});
const update = privateNamesMap.has(name)
? privateNamesMap.get(name)
: {
id: prop.scope.generateUidIdentifier(name),
static: !isInstance,
method: isMethod,
};
if (prop.node.kind === "get") {
update.getId = prop.scope.generateUidIdentifier(`get_${name}`);
} else if (prop.node.kind === "set") {
update.setId = prop.scope.generateUidIdentifier(`set_${name}`);
} else if (prop.node.kind === "method" && isMethod && isInstance) {
update.methodId = prop.scope.generateUidIdentifier(name);
}
privateNamesMap.set(name, update);
}
}
return privateNamesMap;
Expand All @@ -31,15 +41,19 @@ export function buildPrivateNamesNodes(privateNamesMap, loose, state) {
// In spec mode, only instance fields need a "private name" initializer
// because static fields are directly assigned to a variable in the
// buildPrivateStaticFieldInitSpec function.
const { id, static: isStatic, method: isMethod } = value;
const { id, static: isStatic, method: isMethod, getId, setId } = value;
if (loose) {
initNodes.push(
template.statement.ast`
var ${id} = ${state.addHelper("classPrivateFieldLooseKey")}("${name}")
`,
);
} else if (isMethod && !isStatic) {
initNodes.push(template.statement.ast`var ${id} = new WeakSet();`);
if (getId || setId) {
initNodes.push(template.statement.ast`var ${id} = new WeakMap();`);
} else {
initNodes.push(template.statement.ast`var ${id} = new WeakSet();`);
}
} else if (!isStatic) {
initNodes.push(template.statement.ast`var ${id} = new WeakMap();`);
}
Expand Down Expand Up @@ -121,48 +135,66 @@ const privateNameHandlerSpec = {
static: isStatic,
method: isMethod,
methodId,
getId,
setId,
} = privateNamesMap.get(name);

if (isStatic && !isMethod) {
return t.callExpression(
file.addHelper("classStaticPrivateFieldSpecGet"),
[this.receiver(member), t.cloneNode(classRef), t.cloneNode(id)],
);
} else if (isMethod) {
}
if (isMethod) {
if (getId || setId) {
return t.callExpression(file.addHelper("classPrivateFieldGet"), [
this.receiver(member),
t.cloneNode(id),
]);
}
return t.callExpression(file.addHelper("classPrivateMethodGet"), [
this.receiver(member),
t.cloneNode(id),
t.cloneNode(methodId),
]);
} else {
return t.callExpression(file.addHelper("classPrivateFieldGet"), [
this.receiver(member),
t.cloneNode(id),
]);
}
return t.callExpression(file.addHelper("classPrivateFieldGet"), [
this.receiver(member),
t.cloneNode(id),
]);
},

set(member, value) {
const { classRef, privateNamesMap, file } = this;
const { name } = member.node.property.id;
const { id, static: isStatic, method: isMethod } = privateNamesMap.get(
name,
);
const {
id,
static: isStatic,
method: isMethod,
setId,
} = privateNamesMap.get(name);

if (isStatic && !isMethod) {
return t.callExpression(
file.addHelper("classStaticPrivateFieldSpecSet"),
[this.receiver(member), t.cloneNode(classRef), t.cloneNode(id), value],
);
} else if (isMethod) {
}
if (isMethod) {
if (setId) {
return t.callExpression(file.addHelper("classPrivateFieldSet"), [
this.receiver(member),
t.cloneNode(id),
value,
]);
}
return t.callExpression(file.addHelper("classPrivateMethodSet"), []);
} else {
return t.callExpression(file.addHelper("classPrivateFieldSet"), [
this.receiver(member),
t.cloneNode(id),
value,
]);
}
return t.callExpression(file.addHelper("classPrivateFieldSet"), [
this.receiver(member),
t.cloneNode(id),
value,
]);
},

call(member, args) {
Expand Down Expand Up @@ -255,21 +287,91 @@ function buildPrivateStaticFieldInitSpec(prop, privateNamesMap) {
}

function buildPrivateMethodInitLoose(ref, prop, privateNamesMap) {
const { methodId, id } = privateNamesMap.get(prop.node.key.id.name);
const privateName = privateNamesMap.get(prop.node.key.id.name);
const { methodId, id, getId, setId, initAdded } = privateName;
if (initAdded) return;

if (methodId) {
return template.statement.ast`
Object.defineProperty(${ref}, ${id}, {
// configurable is false by default
// enumerable is false by default
// writable is false by default
value: ${methodId.name}
});
`;
}

return template.statement.ast`
Object.defineProperty(${ref}, ${id}, {
// configurable is false by default
// enumerable is false by default
// writable is false by default
value: ${methodId.name}
if (getId || setId) {
privateNamesMap.set(prop.node.key.id.name, {
...privateName,
initAdded: true,
});
`;

if (getId && setId) {
return template.statement.ast`
Object.defineProperty(${ref}, ${id}, {
// configurable is false by default
// enumerable is false by default
// writable is false by default
get: ${getId.name},
set: ${setId.name}
});
`;
} else if (getId && !setId) {
return template.statement.ast`
Object.defineProperty(${ref}, ${id}, {
// configurable is false by default
// enumerable is false by default
// writable is false by default
get: ${getId.name}
});
`;
} else if (!getId && setId) {
return template.statement.ast`
Object.defineProperty(${ref}, ${id}, {
// configurable is false by default
// enumerable is false by default
// writable is false by default
set: ${setId.name}
});
`;
}
}
}

function buildPrivateInstanceMethodInitSpec(ref, 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;
if (initAdded) return;

if (getId || setId) {
privateNamesMap.set(prop.node.key.id.name, {
...privateName,
initAdded: true,
});

if (getId && setId) {
return template.statement.ast`
${id}.set(${ref}, {
get: ${getId.name},
set: ${setId.name}
});
`;
} else if (getId && !setId) {
return template.statement.ast`
${id}.set(${ref}, {
get: ${getId.name}
});
`;
} else if (!getId && setId) {
return template.statement.ast`
${id}.set(${ref}, {
set: ${setId.name}
});
`;
}
}
return template.statement.ast`${id}.add(${ref})`;
}

Expand Down Expand Up @@ -300,9 +402,37 @@ function buildPublicFieldInitSpec(ref, prop, state) {
}

function buildPrivateInstanceMethodDeclaration(prop, privateNamesMap) {
const { methodId } = privateNamesMap.get(prop.node.key.id.name);
const privateName = privateNamesMap.get(prop.node.key.id.name);
const {
methodId,
getId,
setId,
getterDeclared,
setterDeclared,
} = privateName;
const { params, body } = prop.node;
const methodValue = t.functionExpression(methodId, params, body);
const isGetter = getId && !getterDeclared && params.length === 0;
const isSetter = setId && !setterDeclared && params.length > 0;

if (isGetter) {
privateNamesMap.set(prop.node.key.id.name, {
...privateName,
getterDeclared: true,
});
return t.variableDeclaration("var", [
t.variableDeclarator(getId, methodValue),
]);
}
if (isSetter) {
privateNamesMap.set(prop.node.key.id.name, {
...privateName,
setterDeclared: true,
});
return t.variableDeclaration("var", [
t.variableDeclarator(setId, methodValue),
]);
}

return t.variableDeclaration("var", [
t.variableDeclarator(methodId, methodValue),
Expand Down Expand Up @@ -404,7 +534,7 @@ export function buildFieldsInitNodes(

return {
staticNodes,
instanceNodes,
instanceNodes: instanceNodes.filter(Boolean),
wrapClass(path) {
for (const prop of props) {
prop.remove();
Expand Down
34 changes: 31 additions & 3 deletions packages/babel-helper-create-class-features-plugin/src/index.js
View file
Expand Up @@ -75,11 +75,39 @@ export function createClassFeaturePlugin({

if (path.isPrivate()) {
const { name } = path.node.key.id;
const getName = `get ${name}`;
const setName = `set ${name}`;

if (path.node.kind === "get") {
if (
privateNames.has(getName) ||
(privateNames.has(name) && !privateNames.has(setName))
) {
throw path.buildCodeFrameError("Duplicate private field");
}

privateNames.add(getName).add(name);
} else if (path.node.kind === "set") {
if (
privateNames.has(setName) ||
(privateNames.has(name) && !privateNames.has(getName))
) {
throw path.buildCodeFrameError("Duplicate private field");
}

privateNames.add(setName).add(name);
} else {
if (
(privateNames.has(name) &&
(!privateNames.has(getName) && !privateNames.has(setName))) ||
(privateNames.has(name) &&
(privateNames.has(getName) || privateNames.has(setName)))
) {
throw path.buildCodeFrameError("Duplicate private field");
}

if (privateNames.has(name)) {
throw path.buildCodeFrameError("Duplicate private field");
privateNames.add(name);
}
privateNames.add(name);
}

if (path.isClassMethod({ kind: "constructor" })) {
Expand Down
24 changes: 17 additions & 7 deletions packages/babel-helpers/src/helpers.js
View file
Expand Up @@ -1052,7 +1052,11 @@ helpers.classPrivateFieldGet = helper("7.0.0-beta.0")`
if (!privateMap.has(receiver)) {
throw new TypeError("attempted to get private field on non-instance");
}
return privateMap.get(receiver).value;
var descriptor = privateMap.get(receiver);
if (descriptor.get) {
return descriptor.get.call(receiver);
}
return descriptor.value;
}
`;

Expand All @@ -1062,13 +1066,19 @@ helpers.classPrivateFieldSet = helper("7.0.0-beta.0")`
throw new TypeError("attempted to set private field on non-instance");
}
var descriptor = privateMap.get(receiver);
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");
if (descriptor.set) {
descriptor.set.call(receiver, value);
} 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");
}
descriptor.value = value;
}
descriptor.value = value;
return value;
}
`;
Expand Down

0 comments on commit e8de6fa

Please sign in to comment.