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

--outFile is broken when module import is used #8634

Closed
XorgonTheTypesafe opened this issue May 17, 2016 · 20 comments
Closed

--outFile is broken when module import is used #8634

XorgonTheTypesafe opened this issue May 17, 2016 · 20 comments
Assignees
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@XorgonTheTypesafe
Copy link

TypeScript Version:
1.8.10

Node version :v4.2.2
OS: OSX

Code

Main.ts

/// <reference path="./typings/node/node.d.ts"/>

import * as http from 'http';

namespace Scratch
{
    export class ServerWrapper {

        public constructor() {
            this.server = http.createServer();
        }

        public Start(handler :(request :http.ClientRequest, response :http.ClientResponse)=>void) :void {
            this.server.listen(handler);
        }

        private server : http.Server;
    }
    var s = new Scratch.ServerWrapper();
    s.Start((req,res)=>{});
}

Compile with

tsc ./Main.ts --outFile /dev/stdout

Expected behavior: (what is emitted as Main.js when --outFile is not specified)

"use strict";
var http = require('http');
var Scratch;
(function (Scratch) {
    var ServerWrapper = (function () {
        function ServerWrapper() {
            this.server = http.createServer();
        }
        ServerWrapper.prototype.Start = function (handler) {
            this.server.listen(handler);
        };
        return ServerWrapper;
    }());
    Scratch.ServerWrapper = ServerWrapper;
    var s = new Scratch.ServerWrapper();
    s.Start(function (req, res) { });
})(Scratch || (Scratch = {}));

Actual behavior:

/// <reference path="./typings/node/node.d.ts"/>
@aluanhaddad
Copy link
Contributor

Main.ts describes an external module with no exports. I'm not sure if that's what's causing your issue but you may want to look into that as you seem to be trying to export classes from it.

@XorgonTheTypesafe
Copy link
Author

XorgonTheTypesafe commented May 17, 2016

Why an external module when it is defined via 'namespace' and no --module flag is used ?
And why with no exports when there is an export keyword before 'class ServerWrapper' ?
The behavior is the same with or without the export keyword btw, and I don't actually want to export anything from the Scratch namesapce - the generated Main.js without --outFile is what I'm expecting.

My original requirement is to have ServerWrapper in a separate .ts file, referenced via /// <reference in Main.ts and combined into a single js artifact that would define and invoke it (as opposed to require()-ing it at runtime) but it looks like it cannot be done even with a single .ts file with --outFile ?

@mhegazy
Copy link
Contributor

mhegazy commented May 17, 2016

A module, as defined by the ES 2015 spec, is a file with a top level import or export. you do have one of these: import * as http from 'http'; this causes the compiler to treat it as an external module. please see http://www.typescriptlang.org/docs/handbook/modules.html for more details.

by default the module system is commonjs. if you want to enforce that you do not use modules in inadvertently, use --module none, and you should see errors whenever you use a module syntax.

As for the types issue, you will need to extract the types you are using from the module "http" and define them in an interface in the global namespace, this way you can use them without importing.

Hope that helps.

@mhegazy mhegazy closed this as completed May 17, 2016
@mhegazy mhegazy added the Question An issue which isn't directly actionable in code label May 17, 2016
@XorgonTheTypesafe
Copy link
Author

XorgonTheTypesafe commented May 17, 2016

Thanks for the explanation, this is somewhat unintuitive since |'m targeting es5 rather then es2015 (I believe that's the default but even when I specify it explicitly the behaviour is still the same)
and expected the import to be treated as a typescript construct rather than a javascript construct
How does that explain tsc ./Main.ts creating a valid (commonjs es5) output vs. tsc ./Main.ts --outFile app.js creating an empty one though ?

@mhegazy
Copy link
Contributor

mhegazy commented May 17, 2016

commonjs modules can not be concatenated. they do have different scopes, and that is enforced by the engine at runtime. the compiler will give you an error for using --outFile with --module commonjs as well.

@XorgonTheTypesafe
Copy link
Author

XorgonTheTypesafe commented May 17, 2016

so it does:
error TS6082: Only 'amd' and 'system' modules are supported alongside --outFile.
but if we say that commonjs is the default when no --module is specified then shouldn't it give the same error, rather then emitting an empty file ?

@mhegazy
Copy link
Contributor

mhegazy commented May 17, 2016

well.. i was a bit fuzzy when i said "commonjs" was the default. the system detects that the module is unspecified which is commonJS if you have one file that is a module, and none if you do not. and looks like you did not have any imports in the past, then added one and the behavior changed on you. I would recommend using --module none to avoid such implicit behavior changes in the future.

@XorgonTheTypesafe
Copy link
Author

XorgonTheTypesafe commented May 17, 2016

the system detects that the module is unspecified which is commonJS if you have one file that is a module, and none if you do not.

If each possibility (commonjs,none) when specified explicitly along with --outFile, results in an error message I don't see how it can be considered correct behavior not to do so when it is deduced implicitly, at the very least it would have made it clear to me that tsc considers my code an external module.

@mhegazy
Copy link
Contributor

mhegazy commented May 17, 2016

the reason is for backward compatibility. in the past the default was none. switching to commonjs all the time would have broken users not using modules.

@XorgonTheTypesafe
Copy link
Author

You mean there was a time when --outFile was legal for 'none'/'commonjs' ? Didn't that use to be '--out' ?
That still doesn't explain why giving an error message for --outFile when 'commonjs' is deduced implicitly shouldn't be the expected behavior, emitting an empty file also breaks backward compatibility in such a case if I understand correctly.

@mhegazy
Copy link
Contributor

mhegazy commented May 17, 2016

the original behavior was a bit wacky :).

so the original behavior is in the presence of --module commonjs and --outFile, module code will go to its own file, and non-module code will go to the outFile. do not think any one expected that or expected it to continue working this way though.

@XorgonTheTypesafe
Copy link
Author

So the empty js artifact is where the non-module code was supposed to be and since tsc implicitly decided to treat Main.ts as a commonjs module based on the presence of import there was no non-module code ?

@mhegazy
Copy link
Contributor

mhegazy commented May 17, 2016

correct.

@XorgonTheTypesafe
Copy link
Author

XorgonTheTypesafe commented May 17, 2016

Riiiight.
Ok, I think you will agree that this is very non-intuitive to the point of frustration for people not deeply familiar with the ancient (read: ~2 y.o.) lore.
Between browser/node, different versions of javascript, modules vs. namespaces the typescript modularity story is already very hairy, add to that obsolete examples for previous versions of the language that can be found online.
So can this condition be at least made to emit a warning ?
Specifically: when --outFile is used and a .ts file causes no code to be generated despite defining types and no --module flag is specified.

@mhegazy
Copy link
Contributor

mhegazy commented May 17, 2016

i agree.

@mhegazy mhegazy added Bug A bug in TypeScript and removed Question An issue which isn't directly actionable in code labels May 17, 2016
@mhegazy mhegazy self-assigned this May 17, 2016
@mhegazy mhegazy added this to the TypeScript 2.0 milestone May 17, 2016
@mhegazy mhegazy reopened this May 17, 2016
@aluanhaddad
Copy link
Contributor

the system detects that the module is unspecified which is commonJS if you have one file that is a module, and none if you do not.

I had no idea that was the case. It does seem quite confusing perhaps a compiler warning would help.

@XorgonTheTypesafe
Copy link
Author

Thanks!
So it looks like you've decided to make it an error in 2.0 ?

@amcdnl
Copy link

amcdnl commented Jul 19, 2016

@mhegazy So just to be clear, in 2.0 only System and AMD modules are supported for outFile? Is there any plans to expand this?

My use case is I'd like be able to use TypeScript outFile to package my opensource project for others to import.

@weswigham
Copy link
Member

@amcdnl: If you need a concatenated outfile with other module kinds (which is really just commonjs), I'd reccomend using browserify or webpack to do the job (we use browserify for the compiler itself at present). We prototyped an implementation of a commonjs bundler, but the maintanence burden was too high for something a bunch of other well-know tools already do really well.

@amcdnl
Copy link

amcdnl commented Jul 19, 2016

@weswigham understood, I ended up going with Rollup. Example: https://github.com/swimlane/angular2-data-table/blob/master/package.json#L34

Its probably worth just removing the option all together if thats the overall plan to reduce questions like these ;)

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

No branches or pull requests

5 participants