Support `compileOnSave` for `--out`

basarat commented 9 years ago

I don't use --out. So up for grabs.

I don't recommend --out because : https://github.com/TypeStrong/atom-typescript/blob/master/docs/out.md

basarat commented 9 years ago

From @erikvullings : https://github.com/TypeStrong/atom-typescript/commit/1380a8363f731f52b9258b6ee07f451e3d889df5#commitcomment-10394359

I've promised you an example of my project that uses the -out feature and which has several issues:

it cannot use incremental builds (actually, it builds something, but when I use the resulting js file, it complains that __extends is not defined). I have to do a full build.
since version 1.8.0 of atom-typescript my project doesn't compile at all: although the building message is generated, an error is generated in the console (promise is rejected) and no output is generated. I've copied the stack trace below. Please unzip the attached file and see for yourself.

The attached project should generate a csComp.js file in the dist folder. This is loaded in the csMap https://github.com/TNOCS/csMapclient application to create an Angular SPA map. If you need the front end too, let me know, and I'll send you a copy too.

Hope this helps.

Cheers, Erik

"TypeError: Cannot read property 'file' of undefined↵
at
C:\Users\vullingshjlm\.atom\packages\atom-typescript\dist\main\lang\modules\building.js:31:24↵

at Array.forEach (native)↵
at Object.emitFile
(C:\Users\vullingshjlm\.atom\packages\atom-typescript\dist\main\lang\modules\building.js:30:20)↵

at
C:\Users\vullingshjlm\.atom\packages\atom-typescript\dist\main\lang\projectService.js:233:31↵

at Array.map (native)↵
at Object.build
(C:\Users\vullingshjlm\.atom\packages\atom-typescript\dist\main\lang\projectService.js:232:50)↵

at Child.RequesterResponder.processRequest
(C:\Users\vullingshjlm\.atom\packages\atom-typescript\dist\worker\lib\workerLib.js:38:60)↵

at process.<anonymous>
(C:\Users\vullingshjlm\.atom\packages\atom-typescript\dist\worker\lib\workerLib.js:214:23)↵

at process.emit (events.js:119:17)↵
at handleMessage (child_process.js:326:10)"__proto__:
Object__defineGetter__: function __defineGetter__() { [native code]
}__defineSetter__: function __defineSetter__() { [native code]
}__lookupGetter__: function __lookupGetter__() { [native code]
}__lookupSetter__: function __lookupSetter__() { [native code]
}constructor: function Object() { [native code] }hasOwnProperty: function
hasOwnProperty() { [native code] }isPrototypeOf: function isPrototypeOf() {
[native code] }propertyIsEnumerable: function propertyIsEnumerable() {
[native code] }toLocaleString: function toLocaleString() { [native code]
}toString: function toString() { [native code] }valueOf: function valueOf()
{ [native code] }get __proto__: function __proto__() { [native code] }set
__proto__: function __proto__() { [native code] }

basarat commented 9 years ago

I don't recommend --out because : https://github.com/TypeStrong/atom-typescript/blob/master/docs/out.md

kungfusheep commented 9 years ago

Hello

First of all - Atom Typescript is fantastic, we've been looking into it as a cross platform alternative to visual studio.

On the subject of --out, we have a rapidly growing codebase, currently of around 300+ .ts files over a few different projects/libraries, which relies on the use of --out and the generation of sourcemaps by the typescript compiler.

I'm not sure removing support for it becuase it doesn't fit your personal use-case is the right thing to do. Namespaces suit our codebase, we don't want to use an external module loader and are quite happy with the level of control we get from ordering the combination from _references.ts. Changing the codebase as it is now to use a different scheme for loading & referencing modules all use ATS just isn't feasible.

Do you think the decision to remove support for --out could be reverted given that the TypeScript compiler itself supports it?

Cheers Pete

basarat commented 9 years ago

Thanks for the kind words :rose: Note that I use --out elsewhere too : https://github.com/basarat/typescript-collections but wouldn't do it again.

I'm not sure removing support for it becuase it doesn't fit your personal use-case is the right thing to do

Agreed. That is why this issue is open for grabs ;)

namespaces suit our codebase, we don't want to use an external module loader and are quite happy with the level of control we get from ordering the combination from _references.ts

The TypeScript team doesn't support _references for tsconfig : you'll have to use files and it is going to get messy : https://github.com/Microsoft/TypeScript/issues/2472#issuecomment-85330803

Changing the codebase as it is now to use a different scheme for loading & referencing modules all use ATS just isn't feasible

Please reconsider. I've spent a weekend at work at one point. Not saying you should spend a weekend but consider it a part of a future migration.

Do you think the decision to remove support for --out could be reverted given that the TypeScript compiler itself supports it

I am not removing it. Just warning that we don't have good support for it yet. The editor will work fine otherwise.

csnover commented 9 years ago

I feel compelled to second @basarat here, for your own sake. 300+ and growing files with all manual dependency management is a house of cards just waiting to collapse. Take a couple days to fix it now for your own sake. :)

kungfusheep commented 9 years ago

@basarat Thanks for the reply.

I am not removing it. Just warning that we don't have good support for it yet. The editor will work fine otherwise.

This seems to be contrary to what I'm experiencing. See the screenshot below - it's an error that is generated, not a warning. The build no longer runs as it did in previous versions.

@csnover With all respect, it's difficult for you to pass judgement on the best approach for our software development without knowing what kind of company we are, what the software does and the methodologies we employ in-house. Having dependency loading managed by our own code gives us greater control over a large development team.

Cheers.

Deathspike commented 9 years ago

@basarat @csnover I'm confused why you both would call --out such a bad idea.

Runtime errors; Applications should wait for an equivalent of ready before bootstrapping.
Fast compile; You can compile single files into temp and emit a combined file. VS13 is fast.
Global Scope; Choosing --out means choosing for a var on window. It's not bad; it's a choice.
Hard to analyze; I don't get this at all. Hampering a pipeline for the sake of easier tooling dev? :-1:
Hard to scale; Still no problem with a huge amount of files and VS. TS1.4 seems fast enough.
_references.ts; Is unnecessary if you choose for the ready-approach mentioned earlier.

I prefer the --out method as well, for front-end applications (especially Angular ones). Having to import every single interface and other kind of reference is extremely cumbersome. I just type in the name (ala C#) and expect it to be in my application scope (or say, I do a import Shared = App.Shared for a using or namespace if you will). There is no good reason to discourage it just because it doesn't fit your preference of front-end tools.

nycdotnet commented 9 years ago

I must say I prefer internal modules as well, though I prefer bundling on the back end in a build script rather than with --out.

Bundling on the Back End is the name of my next heavy metal album.

kungfusheep commented 9 years ago

We're currently using --out for debug builds, then production builds take that same output and pass it to a minifier per-project.

I think all this proves is people have different preferences and different workflows.

csnover commented 9 years ago

@basarat @csnover I'm confused why you both would call --out such a bad idea.

A decade of experience doing front-end development professionally, with about half that time consulting for many of the largest companies in the world, with big codebases and big development teams that get themselves into hell-level depths of trouble in part by doing things like hand-managing their thousand JavaScript file dependencies instead of using a module system with automatic dependency resolution. :)

I’m not going to go crazy here to try to convince anyone of anything, just going to point out a couple things that maybe you haven’t considered and leave it at that:

Global Scope; Choosing --out means choosing for a var on window. It's not bad; it's a choice.

It’s bad if you want to try to do a rolling upgrade/refactor (name conflict), or load an older and newer version of your app or a library in the same page (name conflict), or accidentally choose the same name as someone else (name conflict!). This happens more often than you might expect, especially if your company has several teams working independently and then someone decides to try integrating two independently written apps. Modules make it trivial to re-map identifiers at runtime so you can avoid conflicts no matter what happens in the future.

Modules also guarantee that dead code (by static analysis of your dependency tree) can be safely removed; when you expose your code in the global scope you lose an entire path of optimisation because it’s no longer safe to assume that nothing else is going to dynamically access one of your exposed globals.

Finally, in the case of TypeScript, once you introduce a library that doesn’t expose itself globally (expect to see more of this as ES modules become more widely adopted!) you’ll be unable to import it since the act of importing will turn your TS file into an external module.

Hard to scale; Still no problem with a huge amount of files and VS. TS1.4 seems fast enough.

Notwithstanding regeneration of the outfile all the time, source maps are positionally sensitive and run-length encoded so most of the map has to be rebuilt if you do this and use source maps (which you should!). At high-10s to 100s kloc combined it’s going to get slow.

Having to import every single interface and other kind of reference is extremely cumbersome.

Yeah, but so is brushing your teeth. You can get away with not doing both for a while but eventually bad things will happen. :) The good news is that you can create a roll-up module if you really need to and import * as foo from 'rollup' (though again the dead code optimisation caveat applies). I find most people type faster than they think so it’s not really a problem, but of course you may not.

Anyway, it’s obviously up to you, I try to help steer people away from the same mistakes that other people have made in the past, but it is up to you if you know you know better, please definitely keep doing what you are doing!

nycdotnet commented 9 years ago

@csnover This is an awesome write-up. I'm definitely not trying to disagree with you, but here's my take on the other side of the coin.

At the scale you're talking about - multiple teams, hundreds of dependencies, multiple versions of the same dependencies, etc., clearly external modules make sense. TypeScript external modules serve teams at that end of the spectrum very well (and there would essentially be no way to survive without following a pattern like that in regular JS).

Based on your experience, it sounds like you rarely have to work on simple projects anymore. My experience has been mostly in smaller environments with much simpler architectural requirements (mostly corporate/line of business web apps). In those environments, choosing TypeScript external modules introduces several headaches that internal modules simply don't have. TypeScript internal modules "just work" in projects from trivial to medium complexity. It might just be that for developers working on these kinds of apps - modularization itself is a revolutionary concept, never mind bundling like what --out provides!

The way I look at it, using internal modules makes more sense from a YAGNI / KISS perspective until you actually start having the problems you're mentioning. As you've argued, using external modules in a system that is already suffering from complex module interrelationships helps reduce the complexity; however, I would assert that using external modules in a system that doesn't require their functionality increases its complexity. At some point on the graph, those lines intersect, but I don't believe it's at zero.

It's funny - I think we answered the same question on Stack Overflow the other day with largely the same arguments (and someone was mad at me for recommending something "clearly against the best practice"). It's just my opinion.

http://stackoverflow.com/questions/29191097/what-is-the-modularization-story-for-typescript-in-the-browser

Again - thanks for posting this! I think you've made a pretty awesome argument.

basarat commented 9 years ago

The build no longer runs as it did in previous versions.

Sorry. My bad. Tracking : https://github.com/TypeStrong/atom-typescript/issues/244

basarat commented 9 years ago

@Deathspike Added examples. Bootstrapping does not help these examples.

Applications should wait for an equivalent of ready before bootstrapping.

If your code depends on any form of js ordering you will get random errors at runtime.

class inheritance can break at runtime.

Consider foo.ts:

class Foo {

}

and a bar.ts:

class Bar extends Foo {

}

If you fail to compile it in correct order e.g. perhaps alphabetically tsc bar.ts foo.ts the code will compile fine but error at runtime with ReferenceError.

module splitting can fail at runtime.

Consider foo.ts:

module App {
    export var foo = 123;
}

And bar.ts:

module App {
    export var bar = foo + 456;
}

If you fail to compile it in correct order e.g. perhaps alphabetically tsc bar.ts foo.ts the code will compile fine but silently fail at runtime with bar set to NaN.

basarat commented 9 years ago

@Deathspike please see updated https://github.com/TypeStrong/atom-typescript/blob/master/docs/out.md (diff https://github.com/TypeStrong/atom-typescript/commit/f1f478265a31c459b40e41ab2e0d5ef469a003f1)

One thing that I cannot stress enough:

There is no good reason to discourage it just because it doesn't fit your preference of front-end tools.

Its not just tools. Its also for code review and understanding for the next person. You will have a hard time reviewing other people's github projects (and I come across this all the time in C#).

csnover commented 9 years ago

@nycdotnet Thanks for the kind words. FWIW, I would not have felt it necessary to say anything about, say, 5 or 10 TS files… but 300+ certainly qualifies on the “this is a large amount of code” scale :)

Deathspike commented 9 years ago

Thanks for the additional examples @basarat

Now the recommendation has started to make a lot of sense :-)

basarat commented 9 years ago

Here is another one:

Files cannot be compiled in isolation. E.g. consider a.ts:

module M {
  var s = t;
}

Will have different output depending upon whether there is a b.ts of the form:

module M {
  export var t = 5;
}

or

var t = 5;

So a.ts cannot be compiled in isolation.

wojciak commented 9 years ago

Hi,

~~One reason for me to use --out is --out /dev/null. Can I achieve this functionality in some other way? (I don't like dangling .js files polluting my project tree)~~

compileOnSave: false did the trick ;]

Deathspike commented 9 years ago

@wojciak You can also:

Redirect JS and declaration files into a different folder
Git ignore the map/js/declaration files and hide ignored files from the tree

I use the latter ever since the arguments against --out have been made crystal clear.

basarat commented 9 years ago

Build works. Restored with v3.0.5. Sorry for my stubbornness. I actually ended up needing to support this to be able to browse the TypeScript compiler code (built with --out) with atomts.

Order seems to be preserved with files already

Note build is slow but valid. That only leaves:

compileOnSave need to be supported

nycdotnet commented 9 years ago

Sky is falling!!! :-)

Also since files in tsconfig.json preserves order on compile when running with tsc, is there a way to get the language service within atom-typescript to respect that also?

I noticed that if I've ever had a TypeScript file open, if I change the order of files in tsconfig.json, the order is reset to alphabetical on save.

You're the best, Bas.

basarat commented 9 years ago

I noticed that if I've ever had a TypeScript file open, if I change the order of files in tsconfig.json, the order is reset to alphabetical on save.

I just did a quick test on the TypeScript src:

Didn't happen. I guess that is done by filesGlob expansion. I don't see an easy way to support filesGlob if you want to use out. Things will :fire:

nycdotnet commented 9 years ago

You're right. It's controlled by the glob. Actually that's kind of awesome. When I get a chance, I'll document this as a feature.

tsconfig

erikvullings commented 9 years ago

The gulp-order module does do that. You could include an extra section in tsconfig in which you order certain files. The remaining files can follow automatically in alphabetic order.

Sent from my Windows Phone

-----Original Message----- From: "Basarat Ali Syed" notifications@github.com Sent: ‎23/‎04/‎2015 02:35 To: "TypeStrong/atom-typescript" atom-typescript@noreply.github.com Cc: "Erik Vullings" erik.vullings@gmail.com Subject: Re: [atom-typescript] Support compileOnSave for --out (#206)

I noticed that if I've ever had a TypeScript file open, if I change the order of files in tsconfig.json, the order is reset to alphabetical on save. I just did a quick test on the TypeScript src:

Didn't happen. I guess that is done by filesGlob expansion. I don't see an easy way to support filesGlob if you want to use out. Things will
— Reply to this email directly or view it on GitHub.

basarat commented 9 years ago

If you still think its a good idea to use out: Here's today's reason to reconsider https://github.com/Microsoft/TypeScript/pull/2917#issuecomment-96289551

xogeny commented 9 years ago

I'm not well versed in the module story around TypeScript. So forgive my ignorance. But after reading this thread, I'm quite confused.

I'm trying to understand the implications of all of this. What I can't figure out from the comments is the implications between --out vs. --outDir and internal vs. external modules.

If I'm developing code that is primarily used in a web application, it seems like most of these arguments are pushing for external modules. Furthermore, it seems like by not supporting --out, the atom-typescript extension is also pushing people toward external modules.

I guess I'm just confused exactly what the "argument" in this thread is? For example, the issue with ordering with --out...aren't those really arguments about internal modules? I mean without explicit imports (external modules), don't you always have this ordering issue even if you use --outDir (i.e., don't you end up having to worry about the order you process the individual files produced with --outDir?).

Ideally, I'd like to use atom-typescript for a web application. My application isn't currently setup to use requirejs. I agree I could re-architect it to do so. While it seems like (all other things being equal) that is considered a best practice...but is it effectively a necessity?

Thanks.

basarat commented 9 years ago

Yes. Effectively a necessity.

Ps: outDir has nothing to do with any of the discussion here. You are free to use outDir.

pitosalas commented 9 years ago

I am trying to follow this discussion and I am glad to see that I am not the only one befuddled by this question.

I was trying to create a simple test case, for a simple program, that happens to need two just two source files. There are no complicated interlocking dependencies. More like a traditional #include.

class1.ts defines a class called Class1 class2.ts defines a class called Class2 main.ts instantiates a Class1, and a Class2, and then does something.

Whereas all I want to do is to execute the combined source files. Which from what I understand means tsc'ing each one, concatenating the resultant .js files in the right order, and then giving them to node to execute.

Or, I have to learn about modules and exporting etc. and then also have to learn about build systems (gulp or grunt or others - still learning)

I am not arguing in favor or against --out but I am just trying to clarify what my options are in this simple case. Maybe I should keep my whole program in one ts file until I learn modules, exporting and build tools.

basarat commented 9 years ago

Whereas all I want to do is to execute the combined source files. Which from what I understand means tsc'ing each one, concatenating the resultant .js files in the right order, and then giving them to node to execute.

If you are executing with node. Then there is no need to concatenate. Just --module commonjs all three > you get three .js files > and then node main.js .... if you use external modules node will automatically load and exec class1.js and class2.js

basarat commented 8 years ago

Since --out is really a build. Please use option buildOnSave : https://github.com/TypeStrong/atom-typescript/blob/master/docs/tsconfig.md#buildonsave :rose:

TypeStrong / atom-typescript

Support `compileOnSave` for `--out` #206