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

Add support for the new decorators proposal #7976

Merged
merged 2 commits into from
Sep 7, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
639 changes: 639 additions & 0 deletions packages/babel-helpers/src/helpers.js

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions packages/babel-plugin-proposal-decorators/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
],
"dependencies": {
"@babel/helper-plugin-utils": "^7.0.0",
"@babel/helper-replace-supers": "^7.0.0",
"@babel/helper-split-export-declaration": "^7.0.0",
"@babel/plugin-syntax-decorators": "^7.0.0"
},
"peerDependencies": {
Expand Down
20 changes: 10 additions & 10 deletions packages/babel-plugin-proposal-decorators/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,20 @@ import legacyVisitor from "./transformer-legacy";
export default declare((api, options) => {
api.assertVersion(7);

const { legacy = false, decoratorsBeforeExport } = options;
const { legacy = false } = options;
if (typeof legacy !== "boolean") {
throw new Error("'legacy' must be a boolean.");
}

if (legacy !== true) {
throw new Error(
"The new decorators proposal is not supported yet." +
' You must pass the `"legacy": true` option to' +
" @babel/plugin-proposal-decorators",
);
}

if (decoratorsBeforeExport !== undefined) {
const { decoratorsBeforeExport } = options;
Copy link
Member

Choose a reason for hiding this comment

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

nit: do we want to just move this destructuring to line 9?

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 prefer to keep it there alongside with the decoratorsBeforeExport validation.

if (decoratorsBeforeExport === undefined) {
if (!legacy) {
throw new Error(
"The decorators plugin requires a 'decoratorsBeforeExport' option," +
" whose value must be a boolean.",
Copy link
Member

Choose a reason for hiding this comment

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

Can we mention legacy: true in here too so it is more discoverable?

);
}
} else {
if (legacy) {
throw new Error(
"'decoratorsBeforeExport' can't be used with legacy decorators.",
Expand Down
213 changes: 211 additions & 2 deletions packages/babel-plugin-proposal-decorators/src/transformer.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,212 @@
// Not implemented yet
import { types as t, template } from "@babel/core";
import splitExportDeclaration from "@babel/helper-split-export-declaration";
import ReplaceSupers from "@babel/helper-replace-supers";

export default {};
function prop(key, value) {
if (!value) return null;
return t.objectProperty(t.identifier(key), value);
}

function value(body, params = []) {
return t.objectMethod("method", t.identifier("value"), params, body);
Copy link
Member

Choose a reason for hiding this comment

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

How do you feel about generating an object method vs a property containing a function expression here? I usually prefer to generate ES5 code, but what you have is also fine.

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 think that ES5 was a good target for ES2015/ES2016 transforms, but now we can output ES6 since users will either only support browsers which support ES6 or compile it down with preset-env.

Copy link
Member

@Jessidhia Jessidhia Sep 7, 2018

Choose a reason for hiding this comment

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

We still have to be careful with helpers, they still have to be as ES3-compatible as possible; but generated output should be safe to use the latest ES20xx syntax that was current before it is made stage-4 I think.

}

function hasDecorators({ node }) {
if (node.decorators && node.decorators.length > 0) return true;

const body = node.body.body;
for (let i = 0; i < body.length; i++) {
const method = body[i];
if (method.decorators && method.decorators.length > 0) {
return true;
}
}

return false;
}

function extractDecorators({ node }) {
Copy link
Member

Choose a reason for hiding this comment

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

nitpick: From a naming standpoint, takeDecorators might make it clearer that this also clears the decorators.

let result;
if (node.decorators && node.decorators.length > 0) {
result = t.arrayExpression(
node.decorators.map(decorator => decorator.expression),
);
}
node.decorators = undefined;
return result;
}

function getKey(node) {
if (node.computed) {
return node.key;
} else {
return t.stringLiteral(node.key.name || String(node.key.value));
Copy link
Member

Choose a reason for hiding this comment

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

I think explicitness here can be a good thing, e.g.

if (node.computed) {
  return node.key;
} else if (t.isIdentifier(node.key)) {
  return t.stringLiteral(node.key.name);
} else {
  return t.stringLiteral(`${node.key.value}`);
}

}
}

function getSingleElementDefinition(path, superRef, classRef, file) {
const { node, scope } = path;
const isMethod = path.isClassMethod();

if (path.isPrivate()) {
throw path.buildCodeFrameError(
`Private ${
isMethod ? "methods" : "fields"
} in decorated classes are not supported yet.`,
);
}

new ReplaceSupers(
{
methodPath: path,
methodNode: node,
objectRef: classRef,
isStatic: node.static,
superRef,
scope,
file,
},
true,
).replace();

const properties = [
prop("kind", t.stringLiteral(isMethod ? node.kind : "field")),
prop("decorators", extractDecorators(path)),
prop("static", node.static && t.booleanLiteral(true)),
prop("key", getKey(node)),
isMethod
? value(node.body, node.params)
: node.value
? value(template.ast`{ return ${node.value} }`)
: prop("value", scope.buildUndefinedNode()),
].filter(Boolean);

return t.objectExpression(properties);
}

function getElementsDefinitions(path, fId, file) {
const superRef = path.node.superClass || t.identifier("Function");
Copy link
Member

Choose a reason for hiding this comment

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

Is Function here right? It looks like ReplaceSupers already defaults to Function.prototype when there is no super class. Is that not right?


const elements = [];
for (const p of path.get("body.body")) {
if (!p.isClassMethod({ kind: "constructor" })) {
elements.push(getSingleElementDefinition(p, superRef, fId, file));
p.remove();
}
}

return t.arrayExpression(elements);
}

function getConstructorPath(path) {
return path
.get("body.body")
.find(path => path.isClassMethod({ kind: "constructor" }));
}

const bareSupersVisitor = {
CallExpression(path, { initializeInstanceElements }) {
if (path.get("callee").isSuper()) {
path.insertAfter(t.cloneNode(initializeInstanceElements));
}
},
Function(path) {
if (!path.isArrowFunctionExpression()) path.skip();
},
};

function insertInitializeInstanceElements(path, initializeInstanceId) {
const isBase = !path.node.superClass;
const initializeInstanceElements = t.callExpression(initializeInstanceId, [
t.thisExpression(),
]);

const constructorPath = getConstructorPath(path);
if (constructorPath) {
if (isBase) {
constructorPath
.get("body")
.unshiftContainer("body", [
t.expressionStatement(initializeInstanceElements),
]);
} else {
constructorPath.traverse(bareSupersVisitor, {
initializeInstanceElements,
});
}
} else {
const constructor = isBase
? t.classMethod(
"constructor",
t.identifier("constructor"),
[],
t.blockStatement([t.expressionStatement(initializeInstanceElements)]),
)
: t.classMethod(
"constructor",
t.identifier("constructor"),
[t.restElement(t.identifier("args"))],
t.blockStatement([
t.expressionStatement(
t.callExpression(t.Super(), [
t.spreadElement(t.identifier("args")),
]),
),
t.expressionStatement(initializeInstanceElements),
]),
);
path.node.body.body.push(constructor);
}
}

function transformClass(path, file) {
const isDeclaration = path.node.id && path.isDeclaration();
const isStrict = path.isInStrictMode();
const { superClass } = path.node;

path.node.type = "ClassDeclaration";
if (!path.node.id) path.node.id = path.scope.generateUidIdentifier("class");

const initializeId = path.scope.generateUidIdentifier("initialize");
const superId =
superClass &&
path.scope.generateUidIdentifierBasedOnNode(path.node.superClass, "super");

if (superClass) path.node.superClass = superId;

const classDecorators = extractDecorators(path);
const definitions = getElementsDefinitions(path, path.node.id, file);

insertInitializeInstanceElements(path, initializeId);

const expr = template.expression.ast`
${file.addHelper("decorate")}(
Copy link
Member

Choose a reason for hiding this comment

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

It might be nice for users to wrap a try/catch here to give users more feedback, e.g.

function addDecorateHelper(file) {
  try {
    return file.addHelper("decorate");
  } catch (err) {
    if (err.code === "BABEL_HELPER_UNKNOWN") {
      err.message += "\n  @babel/plugin-transform-decorators in non-legacy mode requires @babel/core version ^7.0.1 and you appear to be using an older version";
    }
    throw err;
  }
}

${classDecorators || t.nullLiteral()},
function (${initializeId}, ${superClass ? superId : null}) {
${isStrict ? null : t.stringLiteral("use strict")}
Copy link
Member

Choose a reason for hiding this comment

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

This isn't quite the right structure unfortunately, because use strict should be a t.directive, and also because this will insert it into body instead of the directives array. I don't think there's an easy way to create an optional directive with babel-templates at the moment unfortunately. I think we'd have to inject it manually after the template AST is created.

${path.node}
return { F: ${t.cloneNode(path.node.id)}, d: ${definitions} };
},
${superClass}
)
`;

return isDeclaration ? template.ast`let ${path.node.id} = ${expr}` : expr;
}

export default {
ExportDefaultDeclaration(path) {
let decl = path.get("declaration");
if (!decl.isClassDeclaration() || !hasDecorators(decl)) return;

if (decl.node.id) decl = splitExportDeclaration(path);

decl.replaceWith(transformClass(decl, this.file));
},

Class(path) {
if (hasDecorators(path)) {
path.replaceWith(transformClass(path, this.file));
}
},
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
var el, el1;

@(_ => el = _)
class A {
@(_ => el1 = _)
get foo() { return 1; }

set foo(x) { return 2; }
}

expect(el.elements).toHaveLength(1);

expect(el1).toEqual(expect.objectContaining({
descriptor: expect.objectContaining({
get: expect.any(Function),
set: expect.any(Function)
})
}));

var desc = Object.getOwnPropertyDescriptor(A.prototype, "foo");
expect(desc.get()).toBe(1);
expect(desc.set()).toBe(2);
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
var i = 0;

function getKey() {
return (i++).toString();
}

var desc;

@(_ => desc = _)
class Foo {
[getKey()]() {
return 1;
}

[getKey()]() {
return 2;
}
}

expect(desc.elements).toHaveLength(2);

expect(desc.elements[0].key).toBe("0");
expect(desc.elements[0].descriptor.value()).toBe(1);

expect(desc.elements[1].key).toBe("1");
expect(desc.elements[1].descriptor.value()).toBe(2);

expect(i).toBe(2);
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
@(_ => desc = _)
class Foo {
[getKey()]() {
return 1;
}

[getKey()]() {
return 2;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
let Foo = babelHelpers.decorate([_ => desc = _], function (_initialize) {
"use strict";

class Foo {
constructor() {
_initialize(this);
}

}

return {
F: Foo,
d: [{
kind: "method",
key: getKey(),

value() {
return 1;
}

}, {
kind: "method",
key: getKey(),

value() {
return 2;
}

}]
};
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
var i = 0;
var j = 0;

function getKeyI() {
return (i++).toString();
}
function getKeyJ() {
return (j++).toString();
}

var desc;

@(_ => desc = _)
class Foo {
[getKeyI()]() {
return 1;
}

[getKeyJ()]() {
return 2;
}
}

expect(desc.elements).toHaveLength(1);

expect(desc.elements[0].key).toBe("0");
expect(desc.elements[0].descriptor.value()).toBe(2);

expect(i).toBe(1);
expect(j).toBe(1);
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
@(_ => desc = _)
class Foo {
[getKeyI()]() {
return 1;
}

[getKeyJ()]() {
return 2;
}
}
Loading