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

allowSyntheticDefaultImports doesn't work in TypeScript > 1.8.0 #7518

Closed
kenjiru opened this issue Mar 15, 2016 · 19 comments
Closed

allowSyntheticDefaultImports doesn't work in TypeScript > 1.8.0 #7518

kenjiru opened this issue Mar 15, 2016 · 19 comments
Labels
External Relates to another program, environment, or user action which we cannot control.

Comments

@kenjiru
Copy link

kenjiru commented Mar 15, 2016

It seems that the allowSyntheticDefaultImports option doesn't work with the latest version of the TypeScript compiler. It works as expected with TypeScript 1.8.0.

TypeScript Version:

1.8.3 / 1.8.7

Code
I have the following configuration in tsconfig.json

"compilerOptions": {
    "target": "es6",
    "module": "system",
    "allowSyntheticDefaultImports": true
}

And the following test code:

import classNames from "classnames";
let classNamesResult = classNames(["allowSyntheticDefaultImports", "is", "here"]);

Expected behavior:
classNamesResult == "allowSyntheticDefaultImports is here"

Actual behavior:
The code compiles fine, but it fails at runtime, with the following error in the browser console:

Uncaught TypeError: classnames_1.default is not a function
@billti
Copy link
Member

billti commented Mar 15, 2016

Can you show the difference in compiled code (i.e. what is emitted differently) between the two versions you think is the error.

TypeScript has no runtime support for synthetic defaults, it simply allows for the fact they may be present at runtime. It is the module loader (SystemJS in this case) that creates the synthetic default.

From the above it looks like your loader is not creating the default property (i.e. the synthetic default).

@kenjiru
Copy link
Author

kenjiru commented Mar 15, 2016

Here's the diff between the output:

--- /home/kenjiru/typescript-1.8.0-output.js
+++ /home/kenjiru/typescript-1.8.7-output.js
@@ -46,13 +46,8 @@

    "use strict";

-   var _classnames = __webpack_require__(1);
-   
-   var _classnames2 = _interopRequireDefault(_classnames);
-   
-   function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; }
-   
-   var classNamesResult = (0, _classnames2.default)(["allowSyntheticDefaultImports", "is", "here"]);
+   var classnames_1 = __webpack_require__(1);
+   var classNamesResult = classnames_1.default(["allowSyntheticDefaultImports", "is", "here"]);
    console.log(classNamesResult);

 /***/ },

I've put up a small project showcasing the problem.
https://github.com/kenjiru/react-ts-update

I'm building this using webpack-1.12.14 and tsloader-0.7.2, but this should be irrelevant.

@billti
Copy link
Member

billti commented Mar 15, 2016

I just spent a bunch of time searching our codebase history for _interopRequireDefault, with no luck. Searching our issues, I find issues such as #6296 or #5844 which show this function only as an example of what Babel emits. Are you sure the above wasn't generated by Babel?

@kenjiru
Copy link
Author

kenjiru commented Mar 15, 2016

The output is coming from Babel, sorry I missed this detail. I'm trying to figure out why it's different when changing the compiler.

I assume that both tsc and babel will return the same output for the same input. So a possible culprit here might be the tsloader.

@billti billti added the External Relates to another program, environment, or user action which we cannot control. label Mar 15, 2016
@billti
Copy link
Member

billti commented Mar 15, 2016

Yeah, sorry, but doesn't appear to be us. (From the output, I'm guessing you have WebPack in the mix adding code in your build pipeline also).

@billti billti closed this as completed Mar 15, 2016
@kenjiru
Copy link
Author

kenjiru commented Mar 15, 2016

Looking at the source code of ts-loader, I've noticed that it's using the Compiler API to transpile the ts files.

So it's doing something like this:

var ts = require("typescript");
var content =
        "import classNames from 'classnames';" +
        "let classNamesResult = classNames(['allowSyntheticDefaultImports', 'is', 'here']);" +
        "console.log(classNamesResult);";

var compilerOptions = {
    module: 0,
    jsx: 1,
    target: 2,
    noImplicitAny: false,
    removeComments: false,
    preserveConstEnums: true,
    sourceMap: true,
    allowSyntheticDefaultImports: true
};

var result = ts.transpileModule(content, {compilerOptions: compilerOptions});
console.log(result.outputText);

Now the problem is that with TypeScript 1.8.0 we get this results

import classNames from 'classnames';
let classNamesResult = classNames(['allowSyntheticDefaultImports', 'is', 'here']);
console.log(classNamesResult);
//# sourceMappingURL=module.jsx.map

While with TypeScript 1.8.7 it shows this:

"use strict";
const classnames_1 = require('classnames');
let classNamesResult = classnames_1.default(['allowSyntheticDefaultImports', 'is', 'here']);
console.log(classNamesResult);
//# sourceMappingURL=module.jsx.map

@kenjiru
Copy link
Author

kenjiru commented Mar 15, 2016

Running tsc from the command line also return different results.

./node_modules/typescript/bin/tsc -p src/ --out out.js

src/App.ts

import classNames from "classnames";

let classNamesResult = classNames(["allowSyntheticDefaultImports", "is", "here"]);
console.log(classNamesResult);

tsconfig.json

{
    "compileOnSave": false,
    "filesGlob": [
        "../typings/**/*.*.ts",
        "**/*.{ts}"
    ],
    "compilerOptions": {
        "jsx": "preserve",
        "target": "es6",
        "noImplicitAny": false,
        "removeComments": false,
        "preserveConstEnums": true,
        "sourceMap": true,
        "module": "system",
        "allowSyntheticDefaultImports": true
    },
    "files": [
        "../typings/browser.d.ts",
        "App.ts"
    ]
}

It return the following for TypeScript 1.8.0:

System.register("App", ["classnames"], function(exports_1) {
    "use strict";
    var classnames_1;
    var classNamesResult;
    return {
        setters:[
            function (classnames_1_1) {
                classnames_1 = classnames_1_1;
            }],
        execute: function() {
            classNamesResult = classnames_1.default(["allowSyntheticDefaultImports", "is", "here"]);
            console.log(classNamesResult);
        }
    }
});
//# sourceMappingURL=typescript-1.8.0.js.map

And the following for TypeScript 1.8.7:

System.register("App", ["classnames"], function(exports_1, context_1) {
    "use strict";
    var __moduleName = context_1 && context_1.id;
    var classnames_1;
    var classNamesResult;
    return {
        setters:[
            function (classnames_1_1) {
                classnames_1 = classnames_1_1;
            }],
        execute: function() {
            classNamesResult = classnames_1.default(["allowSyntheticDefaultImports", "is", "here"]);
            console.log(classNamesResult);
        }
    }
});
//# sourceMappingURL=typescript-1.8.7.js.map

@billti
Copy link
Member

billti commented Mar 16, 2016

Is there an issue with the above? The emit looks semantically identical to me.

@mhegazy
Copy link
Contributor

mhegazy commented Mar 16, 2016

For future references the __moduleName is a new addition to the system output, see #6359 for more details.

@kenjiru
Copy link
Author

kenjiru commented Mar 16, 2016

The problem is with the Compiler API results, they are quite different.

The TypeScript 1.8.0 result Babel will transpile the code to (which works fine in the browser):

'use strict';

var _classnames = __webpack_require__(1);

var _classnames2 = _interopRequireDefault(_classnames);

function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; }

var classNamesResult = (0, _classnames2.default)(['allowSyntheticDefaultImports', 'is', 'here']);
console.log(classNamesResult);
//# sourceMappingURL=module.jsx.map

And for the TypeScript 1.8.7 will transpile to:

 "use strict";

var classnames_1 = __webpack_require__(1);
var classNamesResult = classnames_1.default(['allowSyntheticDefaultImports', 'is', 'here']);
console.log(classNamesResult);
//# sourceMappingURL=module.jsx.map

Which will throw the following error in the browser:

App.js:3 Uncaught TypeError: classnames_1.default is not a function

@billti
Copy link
Member

billti commented Mar 17, 2016

Looking at your compiler output, the results are not semantically different. However, backing up to the API sample, the issue likely is the setting:

var compilerOptions = {
    module: 0,
    jsx: 1,
    target: 2,
    noImplicitAny: false,
    removeComments: false,
    preserveConstEnums: true,
    sourceMap: true,
    allowSyntheticDefaultImports: true
};

The above is specifying module: 0, which means none. It used to be if you didn't specify a module kind, but were targeting ES6, the emitted module kind would be ES6. That logic still holds, but we also distinguish now between not setting a module kind, and explicitly setting it to none (0 in this case). The above is probably generating an error during compilation (that your code uses modules but the compiler sets modules to none), which may or may not be visible depending on how that tool uses the API, and but falling back to the new emit default of CommonJS modules.

If the above module setting was set to 5 (for ES2015), or probably even if it was removed all together (so it is undefined instead of a number), it will probably behave like it used to.

@kenjiru
Copy link
Author

kenjiru commented Mar 17, 2016

You are right, this issue is a result of the compiler becoming more strict about the module target and the ts-loader reading the incorrect value from the configuration file. This seems to be fixed in the latest version of ts-loader 0.8.1, which was released a few days ago.

Thank you very much for taking the time to look into this issue and for the detailed explanation.

@gitsupersmecher
Copy link

Is a regression added recently on this issue?

Here is my configuration:

"compilerOptions": { "module": "amd", "moduleResolution": "classic", "noImplicitAny": false, "removeComments": true, "preserveConstEnums": true, "allowJs": true, "sourceMap": true, "target": "es6", "jsx": "react", "outDir": "./build/", "types" :[], "allowUnreachableCode": true, "allowSyntheticDefaultImports": true, "baseUrl": ".", "pretty":true, "rootDirs": [ "./src/Scripts" ] },

and here is the code generated:

const classes = classnames_1.default(CLASS_ROOT, {

for the import:

import classnames from 'classnames';

The same issue I have on the react import.

Thanks in advance for the any help!

@aluanhaddad
Copy link
Contributor

@gitsupersmecher --allowSyntheticDefaultImports doesn't affect emit. If you want it to use it, you need a loader, like SystemJS, or you need to use another transpiler, such as Babel, in addition to TypeScript.

@gitsupersmecher
Copy link

Thanks for the reply. What I am saying is that it does not matter if allowSyntheticDefaultImports is set or not. The output of the tsc -p is wrong. I should not have ".default" in "classnames_1.default".

@RyanCavanaugh
Copy link
Member

You wrote a default import and got a default import. What's incorrect about that?

@gitsupersmecher
Copy link

If I change allowSyntheticDefaultImports: false then the output is the same. Is this expected?

@mhegazy
Copy link
Contributor

mhegazy commented Apr 19, 2017

If I change allowSyntheticDefaultImports: false then the output is the same. Is this expected?

allowSyntheticDefaultImports does not change the output. All what allowSyntheticDefaultImports is tells the compiler that at runtime your loader (e.g. SystemJs) will perform this operation of mapping modules to default imports, and thus avoids the error at build time.

@gitsupersmecher
Copy link

gitsupersmecher commented Apr 20, 2017

Thank you, @mhegazy. Appreciate your answer. I did not know that, and your answer clarified it. And thank you @RyanCavanaugh and @aluanhaddad for your input.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
External Relates to another program, environment, or user action which we cannot control.
Projects
None yet
Development

No branches or pull requests

6 participants