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

Transform TypeScript "declare" fields #10546

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import ReplaceSupers, {
import memberExpressionToFunctions from "@babel/helper-member-expression-to-functions";
import optimiseCall from "@babel/helper-optimise-call-expression";

import * as ts from "./typescript";

export function buildPrivateNamesMap(props) {
const privateNamesMap = new Map();
for (const prop of props) {
Expand Down Expand Up @@ -556,6 +558,8 @@ export function buildFieldsInitNodes(
let needsClassRef = false;

for (const prop of props) {
ts.assertFieldTransformed(prop);

const isStatic = prop.node.static;
const isInstance = !isStatic;
const isPrivate = prop.isPrivate();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// @flow

import type { NodePath } from "@babel/traverse";

export function assertFieldTransformed(path: NodePath) {
// TODO (Babel 8): Also check path.node.definite

if (path.node.declare) {
throw path.buildCodeFrameError(
`TypeScript 'declare' fields must first be transformed by ` +
`@babel/plugin-transform-typescript.\n` +
`If you have already enabled that plugin (or '@babel/preset-typescript'), make sure ` +
`that it runs before any plugin related to additional class features:\n` +
` - @babel/plugin-proposal-class-properties\n` +
` - @babel/plugin-proposal-private-methods\n` +
` - @babel/plugin-proposal-decorators`,
);
}
}
158 changes: 90 additions & 68 deletions packages/babel-plugin-transform-typescript/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,94 @@ function registerGlobalType(programScope, name) {
}

export default declare(
(api, { jsxPragma = "React", allowNamespaces = false }) => {
(
api,
{
jsxPragma = "React",
allowNamespaces = false,
allowDeclareFields = false,
},
) => {
api.assertVersion(7);

const JSX_ANNOTATION_REGEX = /\*?\s*@jsx\s+([^\s]+)/;

const classMemberVisitors = {
field(path) {
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 didn't use the ClassProperty visitor because the class fields transform transforms them using the Class visitor, so they would never be visited in practice.

const { node } = path;

if (!allowDeclareFields && node.declare) {
throw path.buildCodeFrameError(
`The 'declare' modifier is only allowed when the 'allowDeclareFields' option of ` +
`@babel/plugin-transform-typescript or @babel/preset-typescript is enabled.`,
);
}
if (node.definite || node.declare) {
if (node.value) {
throw path.buildCodeFrameError(
`Definietly assigned fields and fields with the 'declare' modifier cannot` +
` be initialized here, but only in the constructor`,
);
}

path.remove();
} else if (!allowDeclareFields && !node.value && !node.decorators) {
path.remove();
}

if (node.accessibility) node.accessibility = null;
if (node.abstract) node.abstract = null;
if (node.readonly) node.readonly = null;
if (node.optional) node.optional = null;
if (node.typeAnnotation) node.typeAnnotation = null;
},
method({ node }) {
if (node.accessibility) node.accessibility = null;
if (node.abstract) node.abstract = null;
if (node.optional) node.optional = null;

// Rest handled by Function visitor
},
constructor(path, classPath) {
// Collects parameter properties so that we can add an assignment
// for each of them in the constructor body
//
// We use a WeakSet to ensure an assignment for a parameter
// property is only added once. This is necessary for cases like
// using `transform-classes`, which causes this visitor to run
// twice.
const parameterProperties = [];
for (const param of path.node.params) {
if (
param.type === "TSParameterProperty" &&
!PARSED_PARAMS.has(param.parameter)
) {
PARSED_PARAMS.add(param.parameter);
parameterProperties.push(param.parameter);
}
}

if (parameterProperties.length) {
const assigns = parameterProperties.map(p => {
let id;
if (t.isIdentifier(p)) {
id = p;
} else if (t.isAssignmentPattern(p) && t.isIdentifier(p.left)) {
id = p.left;
} else {
throw path.buildCodeFrameError(
"Parameter properties can not be destructuring patterns.",
);
}

return template.statement.ast`this.${id} = ${id}`;
});

injectInitialization(classPath, path, assigns);
}
},
Comment on lines +88 to +132
Copy link
Member Author

Choose a reason for hiding this comment

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

The code for method and constructor has just been moved here from the lines before, so that all the class members "visitors" are together.

};

return {
name: "transform-typescript",
inherits: syntaxTypeScript,
Expand Down Expand Up @@ -192,27 +275,6 @@ export default declare(
if (node.definite) node.definite = null;
},

ClassMethod(path) {
const { node } = path;

if (node.accessibility) node.accessibility = null;
if (node.abstract) node.abstract = null;
if (node.optional) node.optional = null;

// Rest handled by Function visitor
},

ClassProperty(path) {
const { node } = path;

if (node.accessibility) node.accessibility = null;
if (node.abstract) node.abstract = null;
if (node.readonly) node.readonly = null;
if (node.optional) node.optional = null;
if (node.definite) node.definite = null;
if (node.typeAnnotation) node.typeAnnotation = null;
},

TSIndexSignature(path) {
path.remove();
},
Expand All @@ -238,54 +300,14 @@ export default declare(
// class transform would transform the class, causing more specific
// visitors to not run.
path.get("body.body").forEach(child => {
const childNode = child.node;

if (t.isClassMethod(childNode, { kind: "constructor" })) {
// Collects parameter properties so that we can add an assignment
// for each of them in the constructor body
//
// We use a WeakSet to ensure an assignment for a parameter
// property is only added once. This is necessary for cases like
// using `transform-classes`, which causes this visitor to run
// twice.
const parameterProperties = [];
for (const param of childNode.params) {
if (
param.type === "TSParameterProperty" &&
!PARSED_PARAMS.has(param.parameter)
) {
PARSED_PARAMS.add(param.parameter);
parameterProperties.push(param.parameter);
}
}

if (parameterProperties.length) {
const assigns = parameterProperties.map(p => {
let id;
if (t.isIdentifier(p)) {
id = p;
} else if (
t.isAssignmentPattern(p) &&
t.isIdentifier(p.left)
) {
id = p.left;
} else {
throw path.buildCodeFrameError(
"Parameter properties can not be destructuring patterns.",
);
}

return template.statement.ast`this.${id} = ${id}`;
});

injectInitialization(path, child, assigns);
if (child.isClassMethod()) {
if (child.node.kind === "constructor") {
classMemberVisitors.constructor(child, path);
} else {
classMemberVisitors.method(child, path);
}
} else if (child.isClassProperty()) {
childNode.typeAnnotation = null;

if (!childNode.value && !childNode.decorators) {
child.remove();
}
classMemberVisitors.field(child, path);
}
});
},
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
class A {
declare x;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"plugins": ["transform-typescript"],
"throws": "The 'declare' modifier is only allowed when the 'allowDeclareFields' option of @babel/plugin-transform-typescript or @babel/preset-typescript is enabled."
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
class A {
x;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"plugins": [["transform-typescript", { "allowDeclareFields": true }]]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
class A {
x;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
class A {
declare x;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"plugins": [["transform-typescript", { "allowDeclareFields": true }]]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
class A {}
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
class C {
public a?: number;
private b: number = 0;
readonly c!: number = 1;
readonly c: number = 1;
@foo d: number;
@foo e: number = 3;
f!: number;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
class A {
declare x;
y;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"plugins": [
"proposal-class-properties",
["transform-typescript", { "allowDeclareFields": true }]
],
"throws": "TypeScript 'declare' fields must first be transformed by @babel/plugin-transform-typescript.\nIf you have already enabled that plugin (or '@babel/preset-typescript'), make sure that it runs before any plugin related to additional class features:\n - @babel/plugin-proposal-class-properties\n - @babel/plugin-proposal-private-methods\n - @babel/plugin-proposal-decorators"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
class A {
declare x;
y;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"plugins": [
["transform-typescript", { "allowDeclareFields": true }],
"proposal-class-properties"
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
function _defineProperty(obj, key, value) { if (key in obj) { Object.defineProperty(obj, key, { value: value, enumerable: true, configurable: true, writable: true }); } else { obj[key] = value; } return obj; }

class A {
constructor() {
_defineProperty(this, "y", void 0);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
class A {
x!;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"plugins": [["transform-typescript", { "allowDeclareFields": true }]]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
class A {}
28 changes: 17 additions & 11 deletions packages/babel-preset-typescript/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,13 @@ import transformTypeScript from "@babel/plugin-transform-typescript";
export default declare(
(
api,
{ jsxPragma, allExtensions = false, isTSX = false, allowNamespaces },
{
jsxPragma,
allExtensions = false,
isTSX = false,
allowNamespaces,
allowDeclareFields,
},
) => {
api.assertVersion(7);

Expand All @@ -19,32 +25,32 @@ export default declare(
throw new Error("isTSX:true requires allExtensions:true");
}

const pluginOptions = isTSX => ({
jsxPragma,
isTSX,
allowNamespaces,
allowDeclareFields,
});

return {
overrides: allExtensions
? [
{
plugins: [
[transformTypeScript, { jsxPragma, isTSX, allowNamespaces }],
],
plugins: [[transformTypeScript, pluginOptions(isTSX)]],
},
]
: [
{
// Only set 'test' if explicitly requested, since it requires that
// Babel is being called`
test: /\.ts$/,
plugins: [[transformTypeScript, { jsxPragma, allowNamespaces }]],
plugins: [[transformTypeScript, pluginOptions(false)]],
},
{
// Only set 'test' if explicitly requested, since it requires that
// Babel is being called`
test: /\.tsx$/,
plugins: [
[
transformTypeScript,
{ jsxPragma, isTSX: true, allowNamespaces },
],
],
plugins: [[transformTypeScript, pluginOptions(true)]],
},
],
};
Expand Down