Skip to content

Commit

Permalink
[babel 8] Disallow synchronous usage of babel.* callback methods (#…
Browse files Browse the repository at this point in the history 
…12695)

* [babel 8] Disallow synchronous usage of `babel.{parse,transform{,FromAst,File}}`

Disallow babel.transformFromAst synchronously

Disallow using babel.transform, babel.transformFile, babel.parse synchronously

chore: throw error behind BABEL_8_BREAKING flag

fix: use transformFromAstSync

fix: use transformSync

refactor: use Babel.transformSync in standalone

fix: use parseSync

feat: emit deprecation message on callback-less usage

chore: add tests on thrown error

call sync method on undefined callback

review comments

address review comments

chore: use loadOptionsSync for OptionManager

Co-authored-by: Huáng Jùnliàng <jlhwung@gmail.com>
Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>

* Comment out warning for now

Co-authored-by: Pranav <pranav2000joglekar@gmail.com>
Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>
  • Loading branch information
@JLHwung @Pranav2612000 @nicolo-ribaudo
3 people committed Jun 5, 2022
1 parent ad19401 commit 2ce56e8
Show file tree
Hide file tree
Showing 19 changed files with 225 additions and 138 deletions.
2 changes: 1 addition & 1 deletion packages/babel-core/src/config/full.ts
View file
Expand Up @@ -430,7 +430,7 @@ const validateIfOptionNeedsFilename = (
[
`Preset ${formattedPresetName} requires a filename to be set when babel is called directly,`,
`\`\`\``,
`babel.transform(code, { filename: 'file.ts', presets: [${formattedPresetName}] });`,
`babel.transformSync(code, { filename: 'file.ts', presets: [${formattedPresetName}] });`,
`\`\`\``,
`See https://babeljs.io/docs/en/options#filename for more information.`,
].join("\n"),
Expand Down
4 changes: 2 additions & 2 deletions packages/babel-core/src/index.ts
View file
Expand Up @@ -63,10 +63,10 @@ export const DEFAULT_EXTENSIONS = Object.freeze([
] as const);

// For easier backward-compatibility, provide an API like the one we exposed in Babel 6.
import { loadOptions } from "./config";
import { loadOptionsSync } from "./config";
export class OptionManager {
init(opts: {}) {
return loadOptions(opts);
return loadOptionsSync(opts);
}
}

Expand Down
15 changes: 12 additions & 3 deletions packages/babel-core/src/parse.ts
View file
Expand Up @@ -39,9 +39,18 @@ export const parse: Parse = function parse(code, opts?, callback?) {
opts = undefined;
}

// For backward-compat with Babel 7's early betas, we allow sync parsing when
// no callback is given. Will be dropped in some future Babel major version.
if (callback === undefined) return parseRunner.sync(code, opts);
if (callback === undefined) {
if (process.env.BABEL_8_BREAKING) {
throw new Error(
"Starting from Babel 8.0.0, the 'parse' function expects a callback. If you need to call it synchronously, please use 'parseSync'.",
);
} else {
// console.warn(
// "Starting from Babel 8.0.0, the 'parse' function will expect a callback. If you need to call it synchronously, please use 'parseSync'.",
// );
return parseRunner.sync(code, opts);
}
}

parseRunner.errback(code, opts, callback);
};
Expand Down
13 changes: 10 additions & 3 deletions packages/babel-core/src/transform-ast.ts
View file
Expand Up @@ -45,10 +45,17 @@ export const transformFromAst: TransformFromAst = function transformFromAst(
opts = undefined;
}

// For backward-compat with Babel 6, we allow sync transformation when
// no callback is given. Will be dropped in some future Babel major version.
if (callback === undefined) {
return transformFromAstRunner.sync(ast, code, opts);
if (process.env.BABEL_8_BREAKING) {
throw new Error(
"Starting from Babel 8.0.0, the 'transformFromAst' function expects a callback. If you need to call it synchronously, please use 'transformFromAstSync'.",
);
} else {
// console.warn(
// "Starting from Babel 8.0.0, the 'transformFromAst' function will expect a callback. If you need to call it synchronously, please use 'transformFromAstSync'.",
// );
return transformFromAstRunner.sync(ast, code, opts);
}
}

transformFromAstRunner.errback(ast, code, opts, callback);
Expand Down
15 changes: 12 additions & 3 deletions packages/babel-core/src/transform.ts
View file
Expand Up @@ -31,9 +31,18 @@ export const transform: Transform = function transform(code, opts?, callback?) {
opts = undefined;
}

// For backward-compat with Babel 6, we allow sync transformation when
// no callback is given. Will be dropped in some future Babel major version.
if (callback === undefined) return transformRunner.sync(code, opts);
if (callback === undefined) {
if (process.env.BABEL_8_BREAKING) {
throw new Error(
"Starting from Babel 8.0.0, the 'transform' function expects a callback. If you need to call it synchronously, please use 'transformSync'.",
);
} else {
// console.warn(
// "Starting from Babel 8.0.0, the 'transform' function will expect a callback. If you need to call it synchronously, please use 'transformSync'.",
// );
return transformRunner.sync(code, opts);
}
}

transformRunner.errback(code, opts, callback);
};
Expand Down
104 changes: 82 additions & 22 deletions packages/babel-core/test/api.js
View file
Expand Up @@ -25,10 +25,18 @@ function parse(code, opts) {
return babel.parse(code, { cwd, configFile: false, ...opts });
}

function parseSync(code, opts) {
return babel.parseSync(code, { cwd, configFile: false, ...opts });
}

function transform(code, opts) {
return babel.transform(code, { cwd, configFile: false, ...opts });
}

function transformSync(code, opts) {
return babel.transformSync(code, { cwd, configFile: false, ...opts });
}

function transformFile(filename, opts, cb) {
return babel.transformFile(filename, { cwd, configFile: false, ...opts }, cb);
}
Expand All @@ -51,6 +59,14 @@ function transformFromAst(ast, code, opts) {
return babel.transformFromAst(ast, code, { cwd, configFile: false, ...opts });
}

function transformFromAstSync(ast, code, opts) {
return babel.transformFromAstSync(ast, code, {
cwd,
configFile: false,
...opts,
});
}

describe("parser and generator options", function () {
const recast = {
parse: function (code, opts) {
Expand All @@ -62,7 +78,7 @@ describe("parser and generator options", function () {
};

function newTransform(string) {
return transform(string, {
return transformSync(string, {
ast: true,
parserOpts: {
parser: recast.parse,
Expand All @@ -78,7 +94,7 @@ describe("parser and generator options", function () {
it("options", function () {
const string = "original;";
expect(newTransform(string).ast).toEqual(
transform(string, { ast: true }).ast,
transformSync(string, { ast: true }).ast,
);
expect(newTransform(string).code).toBe(string);
});
Expand All @@ -87,7 +103,7 @@ describe("parser and generator options", function () {
const experimental = "var a: number = 1;";

expect(newTransform(experimental).ast).toEqual(
transform(experimental, {
transformSync(experimental, {
ast: true,
parserOpts: {
plugins: ["flow"],
Expand All @@ -97,7 +113,7 @@ describe("parser and generator options", function () {
expect(newTransform(experimental).code).toBe(experimental);

function newTransformWithPlugins(string) {
return transform(string, {
return transformSync(string, {
ast: true,
plugins: [cwd + "/../../babel-plugin-syntax-flow"],
parserOpts: {
Expand All @@ -110,7 +126,7 @@ describe("parser and generator options", function () {
}

expect(newTransformWithPlugins(experimental).ast).toEqual(
transform(experimental, {
transformSync(experimental, {
ast: true,
parserOpts: {
plugins: ["flow"],
Expand All @@ -124,7 +140,7 @@ describe("parser and generator options", function () {
const experimental = "if (true) {\n import a from 'a';\n}";

expect(newTransform(experimental).ast).not.toBe(
transform(experimental, {
transformSync(experimental, {
ast: true,
parserOpts: {
allowImportExportEverywhere: true,
Expand Down Expand Up @@ -156,6 +172,27 @@ describe("api", function () {
expect(babel.tokTypes).toBeDefined();
});

(process.env.BABEL_8_BREAKING ? it : it.skip)(
"parse throws on undefined callback",
() => {
expect(() => parse("", {})).toThrowErrorMatchingInlineSnapshot(
`"Starting from Babel 8.0.0, the 'parse' function expects a callback. If you need to call it synchronously, please use 'parseSync'."`,
);
},
);

(process.env.BABEL_8_BREAKING ? it : it.skip)(
"transform throws on undefined callback",
() => {
const options = {
filename: "example.js",
};
expect(() => transform("", options)).toThrowErrorMatchingInlineSnapshot(
`"Starting from Babel 8.0.0, the 'transform' function expects a callback. If you need to call it synchronously, please use 'transformSync'."`,
);
},
);

it("transformFile", function () {
const options = {
babelrc: false,
Expand Down Expand Up @@ -189,6 +226,16 @@ describe("api", function () {
// keep user options untouched
expect(options).toEqual({ babelrc: false });
});
it("transformFile throws on undefined callback", () => {
const options = {
babelrc: false,
};
expect(() =>
transformFile(cwd + "/fixtures/api/file.js", options),
).toThrowErrorMatchingInlineSnapshot(
`"Asynchronous function called without callback"`,
);
});

it("transformFileSync", function () {
const options = {
Expand All @@ -201,10 +248,23 @@ describe("api", function () {
expect(options).toEqual({ babelrc: false });
});

it("transformFromAst should not mutate the AST", function () {
(process.env.BABEL_8_BREAKING ? it : it.skip)(
"transformFromAst throws on undefined callback",
() => {
const program = "const identifier = 1";
const node = parseSync(program);
expect(() =>
transformFromAst(node, program),
).toThrowErrorMatchingInlineSnapshot(
`"Starting from Babel 8.0.0, the 'transformFromAst' function expects a callback. If you need to call it synchronously, please use 'transformFromAstSync'."`,
);
},
);

it("transformFromAstSync should not mutate the AST", function () {
const program = "const identifier = 1";
const node = parse(program);
const { code } = transformFromAst(node, program, {
const node = parseSync(program);
const { code } = transformFromAstSync(node, program, {
plugins: [
function () {
return {
Expand All @@ -225,10 +285,10 @@ describe("api", function () {
);
});

it("transformFromAst should mutate the AST when cloneInputAst is false", function () {
it("transformFromAstSync should mutate the AST when cloneInputAst is false", function () {
const program = "const identifier = 1";
const node = parse(program);
const { code } = transformFromAst(node, program, {
const node = parseSync(program);
const { code } = transformFromAstSync(node, program, {
cloneInputAst: false,
plugins: [
function () {
Expand All @@ -252,7 +312,7 @@ describe("api", function () {

it("options throw on falsy true", function () {
return expect(function () {
transform("", {
transformSync("", {
plugins: [cwd + "/../../babel-plugin-syntax-jsx", false],
});
}).toThrow(/.plugins\[1\] must be a string, object, function/);
Expand All @@ -273,7 +333,7 @@ describe("api", function () {
let calledRaw = 0;
let calledIntercept = 0;

transform("function foo() { bar(foobar); }", {
transformSync("function foo() { bar(foobar); }", {
wrapPluginVisitorMethod: function (pluginAlias, visitorType, callback) {
if (pluginAlias !== "foobar") {
return callback;
Expand Down Expand Up @@ -307,7 +367,7 @@ describe("api", function () {
let aliasBaseType = null;

function execTest(passPerPreset) {
return transform("type Foo = number; let x = (y): Foo => y;", {
return transformSync("type Foo = number; let x = (y): Foo => y;", {
sourceType: "script",
passPerPreset: passPerPreset,
presets: [
Expand Down Expand Up @@ -390,7 +450,7 @@ describe("api", function () {
const oldEnv = process.env.BABEL_ENV;
process.env.BABEL_ENV = "development";

const result = transform("", {
const result = transformSync("", {
cwd: path.join(cwd, "fixtures", "config", "complex-plugin-config"),
filename: path.join(
cwd,
Expand Down Expand Up @@ -448,7 +508,7 @@ describe("api", function () {

it("interpreter directive backward-compat", function () {
function doTransform(code, preHandler) {
return transform(code, {
return transformSync(code, {
plugins: [
{
pre: preHandler,
Expand Down Expand Up @@ -501,7 +561,7 @@ describe("api", function () {
});

it("source map merging", function () {
const result = transform(
const result = transformSync(
[
/* eslint-disable max-len */
'function _classCallCheck(instance, Constructor) { if (!(instance instanceof Constructor)) { throw new TypeError("Cannot call a class as a function"); } }',
Expand Down Expand Up @@ -710,7 +770,7 @@ describe("api", function () {
});

it("default", function () {
const result = transform("foo;", {
const result = transformSync("foo;", {
env: {
development: { comments: false },
},
Expand All @@ -721,7 +781,7 @@ describe("api", function () {

it("BABEL_ENV", function () {
process.env.BABEL_ENV = "foo";
const result = transform("foo;", {
const result = transformSync("foo;", {
env: {
foo: { comments: false },
},
Expand All @@ -731,7 +791,7 @@ describe("api", function () {

it("NODE_ENV", function () {
process.env.NODE_ENV = "foo";
const result = transform("foo;", {
const result = transformSync("foo;", {
env: {
foo: { comments: false },
},
Expand Down Expand Up @@ -847,7 +907,7 @@ describe("api", function () {
},
],
}),
).toThrow();
).toThrowErrorMatchingInlineSnapshot(`"unknown: Unknown helper fooBar"`);
});
});
});
8 changes: 4 additions & 4 deletions packages/babel-core/test/parse.js
View file
@@ -1,6 +1,6 @@
import fs from "fs";
import path from "path";
import { parse } from "../lib/index.js";
import { parseSync } from "../lib/index.js";
import { fileURLToPath } from "url";
import { createRequire } from "module";

Expand All @@ -15,12 +15,12 @@ function fixture(...args) {
);
}

describe("parse", function () {
describe("parseSync", function () {
it("should parse using configuration from .babelrc when a filename is provided", function () {
const input = fs.readFileSync(fixture("input.js"), "utf8");
const output = require(fixture("output"));

const result = parse(input, {
const result = parseSync(input, {
filename: fixture("input.js"),
cwd: fixture(),
});
Expand All @@ -31,7 +31,7 @@ describe("parse", function () {
const input = fs.readFileSync(fixture("input.js"), "utf8");
const output = require(fixture("output.json"));

const result = parse(input, {
const result = parseSync(input, {
parserOpts: {
plugins: [["decorators", { decoratorsBeforeExport: false }]],
},
Expand Down

0 comments on commit 2ce56e8

Please sign in to comment.