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
23 changes: 17 additions & 6 deletions src/transformers/CJSImportTransformer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,20 +177,31 @@ 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) {
Copy link
Owner

Choose a reason for hiding this comment

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

=== please. (This breaks lint, and you should be able to do yarn test locally to catch it.)

Also, maybe you could do this.tokens.currentIndex() + 1 === possibleOpenParenIndex?

I think you should be able to handle the new issues by just making sure the prior token is not new in this conditional.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I live on the edge. Will fix it. (regarding yarn test: I've been using jest -i with a jest.config.js I setup, because I can't stand mocha)

Also, maybe you could do this.tokens.currentIndex() + 1 === possibleOpenParenIndex?

Nice! At this point, we could just merge it and you fix the nit picks. :P

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
58 changes: 50 additions & 8 deletions test/imports-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -388,11 +388,53 @@ 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)();
`,
`"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))();
`,
);
});

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 +495,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 +749,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 +808,7 @@ module.exports = exports.default;
return \`interpolated \${value}\`;
}
m2(id) {
(0, _things.foo)();
_things.foo.call(void 0, );
}
}
`,
Expand Down Expand Up @@ -801,7 +843,7 @@ module.exports = exports.default;
var _a = require('a');

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

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

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