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

Support 'target':'es5' with 'module':'es6' to allow dead code elimination #6319

Closed
SynerG opened this issue Jan 2, 2016 · 11 comments
Closed
Assignees
Labels
Committed The team has roadmapped this issue Fixed A PR has been merged for this issue Suggestion An idea for TypeScript

Comments

@SynerG
Copy link

SynerG commented Jan 2, 2016

The import/export statements used in ES6 modules allow tools such as Webpack and Rollup.js to eliminate all the code from unused exports. (For example, see Dr. Axel Rauschmayer's post on this topic)

However, targeting ES5 remove the import/export statements, so these tools cannot remove unused exports.

TypeScript 1.7 included 'es6' and 'es2015' as valid options for the 'module' setting, but the following config:

// tsconfig.json
{
  "compilerOptions": {
    "module": "es6",
    "target": "es5"
  }
}

Produces this error:

error TS1204: Cannot compile modules into 'es6' when targeting 'ES5' or lower.

So, it looks like it is not currently possible to generate ES5 code while retaining the import/export statements of ES6 modules.

This is related to issue #4692 that proposes enabling more granular targeting beyond 'ES5' or 'ES6', but in this particular case the two required options already exist in tsconfig.json ('target' and 'module'), but the particular combination of values ('target':'es5', 'module':'es6') doesn't work.

@pkozlowski-opensource
Copy link

Also see #5806 and #5790

@SynerG
Copy link
Author

SynerG commented Jan 4, 2016

@pkozlowski-opensource thanks for the two relevant related issues.

#5806 presented the same issue, but it was closed in favor of #5790 to prevent redundancy.
But #5790 was closed prematurely because the initial post was not clear enough and was misinterpreted as trying to transpile ilegal ES6 code.

(#5790 contains relevant discussion on the topic. The real issue was clarified after being closed. @weswigham stated that 'if enough people see a need for it, we could conceivably support the configuration - it's just not something our emitter currently expects to need to do' and @kitsonk said that 'there is no harm in emitting ES5 code in ES6 module format (though my opinion is far from official)'. The issue also mentions that UglifyJS and Google Closure Compiler do not implement the same tree-shaking capabilities of Rollup/Webpack 2)

So, the 2 previous issues were closed for reasons beyond the issue itself.
But there is not possible yet to transpile to ES5 using ES6 module syntax, or combine TypeScript with tree-shaking tools that remove unused exports (Webpack 2/Rollup) without requiring another tool such as Babel.
I will keep this issue open for now. (#5790 could be reopened to keep the old discussion).

@mhegazy mhegazy added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Jan 5, 2016
@frankwallis
Copy link
Contributor

This issue currently prevents using the systemjs-builder rollup support unless you also use babel in the pipeline to output es6 modules with es5 code. See frankwallis/plugin-typescript#97 (comment)

So +1 from me, thanks.

@JabX
Copy link

JabX commented Apr 8, 2016

👍

Another issue for me is that Typescript's module resolution is pretty much only compatible with true ES6 modules, which means that using good old CommonJS modules (like from npm) is problematic when you want to get the default export of one of those modules. The workaround is to import them as import * as module from 'module', but it's against the ES6 standard and doesn't work well with module definitions.

This means that if using this workaround isn't acceptable (in my case it is, so I'm fine), you are today forced to compile to ES6 and chain with Babel to transpile everything, whereas you could have simply compiled your ES6 to ES5 with Typescript and used Babel's module transform (or even directly Webpack 2 that works the same).

@mhegazy
Copy link
Contributor

mhegazy commented Apr 28, 2016

@JabX consider using --allowSyntheticDefaultImports, if you are sure the module you are importing has a default export. see https://github.com/Microsoft/TypeScript/wiki/What%27s-new-in-TypeScript#support-for-default-import-interop-with-systemjs

@JabX
Copy link

JabX commented Apr 29, 2016

@mhegazy That flag is only about getting the type definitions right. The problem lies in the transpiled code.

Writing import Module from 'module' will generate something like var Module = require('module').default, whereas import * as Module from 'module' generates var Module = require('module').

These are not the same, and only the second works with CommonJS.

@frankwallis
Copy link
Contributor

frankwallis commented Apr 29, 2016

You can also use:

import module = require('module');

to import commonjs

@JabX
Copy link

JabX commented Apr 29, 2016

I know about this syntax, but you can't use it in JS files, which I also transpile using Typescript. I should have said so earlier.

@guybedford
Copy link
Contributor

guybedford commented May 4, 2016

This is critical to the Rollup integration with TypeScript. In the eventual future as well, when ES modules are supported natively, this option will have become the default output, so there is no risk to implementation as it will be needed one day anyway.

@guybedford
Copy link
Contributor

Oh I see module: es6 + target: es6 will work, so scratch the above argument. Still important for Rollup support though!

@texastoland
Copy link
Contributor

texastoland commented May 17, 2016

I'm currently doing TS -> ES6 -> rollup -> ES5 via tsc. (source)

I can't find the justification why we should compile twice in order to use a separate bundler?

Edit: partially works using the Compiler API. @Victorystick implemented a workaround) for export class.

$ node -e 'const ts=require("typescript");console.log(ts.transpileModule("export const cube=x=>x*x*x",{reportDiagnostics:true,compilerOptions:{module:ts.ModuleKind.ES2015}}))'
{ outputText: 'export var cube = function (x) { return x * x * x; };\n',
  diagnostics: 
   [ { file: undefined,
       start: undefined,
       length: undefined,
       messageText: 'Cannot compile modules into \'es2015\' when targeting \'ES5\' or lower.',
       category: 1,
       code: 1204 } ],
  sourceMapText: undefined }

$ node -e 'const ts=require("typescript");console.log(ts.transpileModule("export const cube=x=>x*x*x",{compilerOptions:{module:ts.ModuleKind.ES2015}}))'
{ outputText: 'export var cube = function (x) { return x * x * x; };\n',
  diagnostics: undefined,
  sourceMapText: undefined }

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Committed The team has roadmapped this issue Fixed A PR has been merged for this issue Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

7 participants