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.

Sign up for GitHub

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

Jump to bottom

Add support for --noEmitHelpers flag #2901

Merged
merged 1 commit into from
May 1, 2015
Merged

Add support for --noEmitHelpers flag #2901

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions bin/typescript.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1087,6 +1087,7 @@ declare module "typescript" {
mapRoot?: string;
module?: ModuleKind;
noEmit?: boolean;
noEmitHelpers?: boolean;
noEmitOnError?: boolean;
noErrorTruncation?: boolean;
noImplicitAny?: boolean;
Expand Down
1 change: 1 addition & 0 deletions bin/typescriptServices.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1087,6 +1087,7 @@ declare module ts {
mapRoot?: string;
module?: ModuleKind;
noEmit?: boolean;
noEmitHelpers?: boolean;
noEmitOnError?: boolean;
noErrorTruncation?: boolean;
noImplicitAny?: boolean;
Expand Down
4 changes: 4 additions & 0 deletions src/compiler/commandLineParser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ module ts {
type: "boolean",
description: Diagnostics.Do_not_emit_outputs,
},
{
name: "noEmitHelpers",
type: "boolean"
},
{
name: "noEmitOnError",
type: "boolean",
Expand Down
42 changes: 27 additions & 15 deletions src/compiler/emitter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5487,6 +5487,9 @@ if (typeof __param !== "function") __param = function (paramIndex, decorator) {
}

write("[\"require\", \"exports\"");
if (compilerOptions.noEmitHelpers) {
Copy link
Member

Choose a reason for hiding this comment

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

We are considering a different approach to making __extends and other helpers available with something like --noEmitHelpers, so I would recommend removing this line.

write(", \"__extends\"");
}
if (aliasedModuleNames.length) {
write(", ");
write(aliasedModuleNames.join(", "));
Expand All @@ -5496,6 +5499,9 @@ if (typeof __param !== "function") __param = function (paramIndex, decorator) {
write(unaliasedModuleNames.join(", "));
}
write("], function (require, exports");
if (compilerOptions.noEmitHelpers) {
Copy link
Member

Choose a reason for hiding this comment

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

We are considering a different approach to making __extends and other helpers available with something like --noEmitHelpers, so I would recommend removing this line.

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 @rbuckton I'm a little confused by that comment. Does this still apply? The intention behind this line is to inject an implementation of said helper that user can tap into.

I'm thinking the check still needs to be there but maybe some logic could be applied to only add in the helper module when needed. Do you agree?

Copy link
Contributor

Choose a reason for hiding this comment

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

This makes it required for users to define "__extends" as a module, only in AMD., but global otherwise. i think we need to make it always global. i.e. remove the addition of "__extends" as a dependency name. If you want to override that you need to define a global var __extends:

something like:

if (typeof __extends !== "function") {
  __extends = function (d, b) {
    for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p];
    function __() { this.constructor = d; }
     __.prototype = b.prototype;
    d.prototype = new __();
  };
}

Copy link
Member

Choose a reason for hiding this comment

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

@whitneyit If you look at #2911, what we are considering is making a separate runtime library available that contains these helpers, which would allow you to write the following:

tsc --noEmitHelpers --import tslib --module commonjs app.ts

So your app.js output might be the following for CommonJS:

require("tslib");
// app body

Or the following for AMD:

define(["require", "exports", "tslib"], function (require, exports) {
  // app body
});

And the tslib library (or whatever it is called), would define globals for __extends, __decorate, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the tslib runtime is going to handle these helpers is there anything from with say using: tslib.extends instead of __extends? That way we could do away with globals in all contexts/module patterns

Copy link
Member

Choose a reason for hiding this comment

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

Currently we have no way of knowing that we should import __extends from a library. We also need to be able to use these globals for <script/> references in the browser. That said, while the approach we're considering does introduce globals, you can still write the following, as the helpers are exported as well:

import { __extends } from 'tslib';

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But you would know if a --module argument has been passed wouldn't you? That's what I was trying to do with the above line:

write(", \"__extends\"");

I was trying to add __extends as a module to the amd pattern. So if we are able to deduce that we are in a module then couldn't we use the tslib.extends pattern and use __extends otherwise. How's that sound?

Example taken from #1350

// commonjs
var __extends = require('./tslib').extends;
var Mammal = require('./mammal');
var Human = (function (_super) {
    __extends(Human, _super);
    function Human() {
        _super.apply(this, arguments);
    }
    return Human;
})(Mammal);

// amd
define(["require", "exports", "tslib", './mammal'], function (require, exports, tslib, Mammal) {
    var Human = (function (_super) {
        tslib.extends(Human, _super);
        function Human() {
            _super.apply(this, arguments);
        }
        return Human;
    })(Mammal);
    return Human;
});

// script
var __extends = this.__extends || function (d, b) {
    for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p];
    function __() { this.constructor = d; }
    __.prototype = b.prototype;
    d.prototype = new __();
};
var Animal = (function () {
    function Animal() {
    }
    return Animal;
})();
var Mammal = (function (_super) {
    __extends(Mammal, _super);
    function Mammal() {
        _super.apply(this, arguments);
    }
    return Mammal;
})(Animal);
var Human = (function (_super) {
    __extends(Human, _super);
    function Human() {
        _super.apply(this, arguments);
    }
    return Human;
})(Mammal);

write(", __extends");
}
if (importAliasNames.length) {
write(", ");
write(importAliasNames.join(", "));
Expand Down Expand Up @@ -5614,24 +5620,30 @@ if (typeof __param !== "function") __param = function (paramIndex, decorator) {

// emit prologue directives prior to __extends
var startIndex = emitDirectivePrologues(node.statements, /*startWithNewLine*/ false);
// Only Emit __extends function when target ES5.
// For target ES6 and above, we can emit classDeclaration as is.
if ((languageVersion < ScriptTarget.ES6) && (!extendsEmitted && resolver.getNodeCheckFlags(node) & NodeCheckFlags.EmitExtends)) {
writeLines(extendsHelper);
extendsEmitted = true;
}

if (!decorateEmitted && resolver.getNodeCheckFlags(node) & NodeCheckFlags.EmitDecorate) {
writeLines(decorateHelper);
if (compilerOptions.emitDecoratorMetadata) {
writeLines(metadataHelper);
// Only emit helpers if the user did not say otherwise.
if (!compilerOptions.noEmitHelpers) {

// Only Emit __extends function when target ES5.
// For target ES6 and above, we can emit classDeclaration as is.
if ((languageVersion < ScriptTarget.ES6) && (!extendsEmitted && resolver.getNodeCheckFlags(node) & NodeCheckFlags.EmitExtends)) {
writeLines(extendsHelper);
extendsEmitted = true;
}

if (!decorateEmitted && resolver.getNodeCheckFlags(node) & NodeCheckFlags.EmitDecorate) {
writeLines(decorateHelper);
if (compilerOptions.emitDecoratorMetadata) {
writeLines(metadataHelper);
}
decorateEmitted = true;
}

if (!paramEmitted && resolver.getNodeCheckFlags(node) & NodeCheckFlags.EmitParam) {
writeLines(paramHelper);
paramEmitted = true;
}
decorateEmitted = true;
}

if (!paramEmitted && resolver.getNodeCheckFlags(node) & NodeCheckFlags.EmitParam) {
writeLines(paramHelper);
paramEmitted = true;
}

if (isExternalModule(node) || compilerOptions.separateCompilation) {
Expand Down
1 change: 1 addition & 0 deletions src/compiler/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1657,6 +1657,7 @@ module ts {
mapRoot?: string;
module?: ModuleKind;
noEmit?: boolean;
noEmitHelpers?: boolean;
noEmitOnError?: boolean;
noErrorTruncation?: boolean;
noImplicitAny?: boolean;
Expand Down
6 changes: 5 additions & 1 deletion src/harness/harness.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -990,6 +990,10 @@ module Harness {
}
break;

case 'noemithelpers':
options.noEmitHelpers = !!setting.value;
break;

case 'noemitonerror':
options.noEmitOnError = !!setting.value;
break;
Expand Down Expand Up @@ -1477,7 +1481,7 @@ module Harness {

// List of allowed metadata names
var fileMetadataNames = ["filename", "comments", "declaration", "module",
"nolib", "sourcemap", "target", "out", "outdir", "noemitonerror",
"nolib", "sourcemap", "target", "out", "outdir", "noemithelpers", "noemitonerror",
"noimplicitany", "noresolve", "newline", "newlines", "emitbom",
"errortruncation", "usecasesensitivefilenames", "preserveconstenums",
"includebuiltfile", "suppressimplicitanyindexerrors", "stripinternal",
Expand Down
19 changes: 19 additions & 0 deletions tests/baselines/reference/noEmitHelpers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
//// [noEmitHelpers.ts]

class A { }
class B extends A { }


//// [noEmitHelpers.js]
var A = (function () {
function A() {
}
return A;
})();
var B = (function (_super) {
__extends(B, _super);
function B() {
_super.apply(this, arguments);
}
return B;
})(A);
9 changes: 9 additions & 0 deletions tests/baselines/reference/noEmitHelpers.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
=== tests/cases/compiler/noEmitHelpers.ts ===

class A { }
>A : Symbol(A, Decl(noEmitHelpers.ts, 0, 0))

class B extends A { }
>B : Symbol(B, Decl(noEmitHelpers.ts, 1, 11))
>A : Symbol(A, Decl(noEmitHelpers.ts, 0, 0))

9 changes: 9 additions & 0 deletions tests/baselines/reference/noEmitHelpers.types
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
=== tests/cases/compiler/noEmitHelpers.ts ===

class A { }
>A : A

class B extends A { }
>B : B
>A : A

4 changes: 4 additions & 0 deletions tests/cases/compiler/noEmitHelpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// @noemithelpers: true

class A { }
class B extends A { }