microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
101.08k stars 12.49k forks source link

Output directory messed up if tsc is triggered as VS Code task. #8837

Closed vilicvane closed 8 years ago

vilicvane commented 8 years ago

This only happens if tsc is executed via Code.

TypeScript Version:

nightly (1.9.0-dev.20160525-1.0)

Code

.vscode/tasks.json

{
    "version": "0.1.0",
    "command": "node", // Using npm scripts would result the same.
    "isShellCommand": true,
    "args": ["node_modules/typescript/lib/tsc.js"],
    "showOutput": "silent",
    "problemMatcher": "$tsc"
}

tsconfig.json

{
    "compilerOptions": {
        "module": "commonjs",
        "allowJs": true,
        "outDir": "bld",
        "rootDir": "src"
    },
    "exclude": [
        "node_modules",
        "bld"
    ]
}

src/foo/bar.ts

import '../biu/pia';

src/biu/pia.js

// Empty file.

Expected behavior:

Compiles correctly.

Actual behavior:

error TS5055: Cannot write file '.../src/biu/pia.js' because it would overwrite input file.

And if you put another .ts file and import it in src/foo.bar.ts, it would be output as .js file nested to its .ts source.

Seikho commented 8 years ago

Possibly related to Microsoft/Vscode#6719 ?

mhegazy commented 8 years ago

@zhengbli any thoughts?

zhengbli commented 8 years ago

Seems to be a VSCode task runner issue with node v6, the same as Microsoft/vscode#6719. Running tsc from command line doesn't repro the error.

Seikho commented 8 years ago

The regression occurs in the cd1af127 commit.

Seikho commented 8 years ago

This (L590, program.ts) seems to be the offending change. It changes the capitalisation of the Windows drive letter (lower to upper in the case of Microsoft/vscode#6719).

When run from the command line, the drive letter is already capitalised and stays capitalised.

yortus commented 8 years ago

Evidence that @Seikho has identified the problem.

Seikho commented 8 years ago

After further digging, it's a change from Node v5 to Node v6 in the behaviour of fs.realpath which is used in L547 sys.ts.

In node v5:

$ node
> process.version
'v5.11.1'
> require('fs').realpathSync('c:\\projects')
'c:\\projects'
>                                     

In node v6:

$ node
> process.version
'v6.2.0'
> require('fs').realpathSync('c:\\projects')
'C:\\projects'
>
yortus commented 8 years ago

@zhengbli could the 'external' tag be removed, and this be dealt with in TypeScript? See https://github.com/Microsoft/vscode/issues/6719#issuecomment-224828922.

EDIT: See also comment from node.js contributor about this: https://github.com/nodejs/node/issues/6624#issuecomment-221558779

dbaeumer commented 8 years ago

Important observations from https://github.com/Microsoft/vscode/issues/6719:

zhengbli commented 8 years ago

@Seikho Thanks for the finding! The issue should be able to address from TypeScript side.

dbaeumer commented 8 years ago

@zhengbli shouldn't this be addressed in 2.0. It works in 1.8.x and is broken in @next. I am asking since it is tagged for 2.1

yortus commented 8 years ago

Fixing in @next would be great - We've had to disable the VSCode build task for all our affected projects since this surfaced, which is a minor hassle but still a hassle.