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

fix: #322 #337

Merged
merged 10 commits into from
Nov 19, 2018
Merged
26 changes: 20 additions & 6 deletions src/transformers/CJSImportTransformer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,20 +177,34 @@ export default class CJSImportTransformer extends Transformer {
if (!replacement) {
return false;
}
// We need to change to (0, f) if this is a function call, so that it won't be interpreted as a
// method access. This can also happen in situations like (f)(), so a following close-paren
// should trigger this behavior as well if it eventually has an open-paren. In some cases, like
// export assignees, we must NOT turn the identifier into a normal expression, so we need to
// just to the regular replacement.
// Tolerate any number of closing parens while looking for an opening paren
// that indicates a function call.
let possibleOpenParenIndex = this.tokens.currentIndex() + 1;
while (
possibleOpenParenIndex < this.tokens.tokens.length &&
this.tokens.tokens[possibleOpenParenIndex].type === tt.parenR
) {
possibleOpenParenIndex++;
}
// Avoid treating imported functions as methods of their `exports` object
// by using `(0, f)` when the identifier is in a paren expression. Else
// use `Function.prototype.call` when the identifier is a guaranteed
// function call. When using `call`, pass undefined as the context.
if (this.tokens.tokens[possibleOpenParenIndex].type === tt.parenL) {
this.tokens.replaceToken(`(0, ${replacement})`);
if (
this.tokens.tokenAtRelativeIndex(1).type === tt.parenL &&
this.tokens.tokenAtRelativeIndex(-1).type !== tt._new
) {
this.tokens.replaceToken(`${replacement}.call(void 0, `);
// Remove the old paren.
this.tokens.removeToken();
// Balance out the new paren.
this.rootTransformer.processBalancedCode();
this.tokens.copyExpectedToken(tt.parenR);
} else {
// See here: http://2ality.com/2015/12/references.html
this.tokens.replaceToken(`(0, ${replacement})`);
}
} else {
this.tokens.replaceToken(replacement);
}
Expand Down
60 changes: 52 additions & 8 deletions test/imports-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -388,11 +388,55 @@ var _moduleName = require('moduleName');
`"use strict";
var _mymodule = require('my-module');

(0, _mymodule.bar)();
_mymodule.bar.call(void 0, );
`,
);
});

it("never breaks weird call expressions", () => {
aleclarson marked this conversation as resolved.
Show resolved Hide resolved
assertResult(
`
import a from 'a';
import b from 'b';
(a)();
((a))();
a(b)();
(a + b)();
new (a)();
Copy link
Owner

Choose a reason for hiding this comment

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

Technically this doesn't exercise your case, since it's surrounded by parens here, but no big deal.

`,
`"use strict";${IMPORT_DEFAULT_PREFIX}
var _a = require('a'); var _a2 = _interopRequireDefault(_a);
var _b = require('b'); var _b2 = _interopRequireDefault(_b);
((0, _a2.default))();
(((0, _a2.default)))();
_a2.default.call(void 0, (0, _b2.default))();
(_a2.default + (0, _b2.default))();
new ((0, _a2.default))();
`,
);
});

describe("ASI interop", () => {
it("prevents accidental invocation for imported functions", () => {
assertResult(
`
import {fun} from 'my-module'

let arr = []
fun()
fun(1, 2)
`,
`"use strict";
var _mymodule = require('my-module');

let arr = []
_mymodule.fun.call(void 0, )
_mymodule.fun.call(void 0, 1, 2)
`,
);
});
});

it("uses wildcard name on default access when possible", () => {
assertResult(
`
Expand Down Expand Up @@ -453,7 +497,7 @@ var _moduleName = require('moduleName');

class A {
constructor() {
this.val = (0, _foo2.default)();
this.val = _foo2.default.call(void 0, );
}
}
`,
Expand Down Expand Up @@ -707,13 +751,13 @@ module.exports = exports.default;

const o = {
f() {
(0, _foo.foo)(3);
_foo.foo.call(void 0, 3);
}
}

class C {
g() {
(0, _foo.foo)(4);
_foo.foo.call(void 0, 4);
}
}
`,
Expand Down Expand Up @@ -766,7 +810,7 @@ module.exports = exports.default;
return \`interpolated \${value}\`;
}
m2(id) {
(0, _things.foo)();
_things.foo.call(void 0, );
}
}
`,
Expand Down Expand Up @@ -801,7 +845,7 @@ module.exports = exports.default;
var _a = require('a');

{
(0, _a.a)();
_a.a.call(void 0, );
}
`,
);
Expand All @@ -823,7 +867,7 @@ module.exports = exports.default;

switch (foo) {
case 1: {
(0, _a.a)();
_a.a.call(void 0, );
}
}
`,
Expand All @@ -846,7 +890,7 @@ module.exports = exports.default;

switch (foo) {
default: {
(0, _a.a)();
_a.a.call(void 0, );
}
}
`,
Expand Down