Skip to content

Plan item #41678 enable @ts-check - #41953

Closed
Erich Gamma (egamma) wants to merge 17 commits into
masterfrom
egamma/issue41678
Closed

Plan item #41678 enable @ts-check #41953
Erich Gamma (egamma) wants to merge 17 commits into
masterfrom
egamma/issue41678

Conversation

@egamma

Copy link
Copy Markdown
Member

No description provided.

@joaomorenoJoão Moreno (joaomoreno) added the engineering VS Code - Build / issue tracking / etc. label Jan 25, 2018
@joaomoreno

João Moreno (joaomoreno) commented Jan 25, 2018

Copy link
Copy Markdown
Contributor

Erich Gamma (@egamma) As Travis and AppVeyor complain, there's a problem with this. The build folder fails to build:


> code-oss-dev-build@1.0.0 compile /Users/joao/Work/vscode/build
> tsc
error TS5055: Cannot write file '/Users/joao/Work/vscode/build/dependencies.js' because it would overwrite input file.
error TS5055: Cannot write file '/Users/joao/Work/vscode/build/gulpfile.editor.js' because it would overwrite input file.
error TS5055: Cannot write file '/Users/joao/Work/vscode/build/gulpfile.extensions.js' because it would overwrite input file.
error TS5055: Cannot write file '/Users/joao/Work/vscode/build/gulpfile.hygiene.js' because it would overwrite input file.
error TS5055: Cannot write file '/Users/joao/Work/vscode/build/gulpfile.mixin.js' because it would overwrite input file.
error TS5055: Cannot write file '/Users/joao/Work/vscode/build/gulpfile.test.js' because it would overwrite input file.
error TS5055: Cannot write file '/Users/joao/Work/vscode/build/gulpfile.vscode.js' because it would overwrite input file.
error TS5055: Cannot write file '/Users/joao/Work/vscode/build/gulpfile.vscode.linux.js' because it would overwrite input file.
error TS5055: Cannot write file '/Users/joao/Work/vscode/build/gulpfile.vscode.win32.js' because it would overwrite input file.
error TS5055: Cannot write file '/Users/joao/Work/vscode/build/lib/builtInExtensions.js' because it would overwrite input file.
error TS5055: Cannot write file '/Users/joao/Work/vscode/build/lib/electron.js' because it would overwrite input file.
error TS5055: Cannot write file '/Users/joao/Work/vscode/build/lib/watch/index.js' because it would overwrite input file.
error TS5055: Cannot write file '/Users/joao/Work/vscode/build/lib/watch/watch-nsfw.js' because it would overwrite input file.
error TS5055: Cannot write file '/Users/joao/Work/vscode/build/lib/watch/watch-win32.js' because it would overwrite input file.
error TS5055: Cannot write file '/Users/joao/Work/vscode/build/npm/postinstall.js' because it would overwrite input file.
error TS5055: Cannot write file '/Users/joao/Work/vscode/build/npm/preinstall.js' because it would overwrite input file.
error TS5055: Cannot write file '/Users/joao/Work/vscode/build/npm/update-all-grammars.js' because it would overwrite input file.
error TS5055: Cannot write file '/Users/joao/Work/vscode/build/npm/update-grammar.js' because it would overwrite input file.
error TS5055: Cannot write file '/Users/joao/Work/vscode/build/npm/update-theme.js' because it would overwrite input file.
error TS5055: Cannot write file '/Users/joao/Work/vscode/src/buildfile.js' because it would overwrite input file.
error TS5055: Cannot write file '/Users/joao/Work/vscode/src/vs/code/buildfile.js' because it would overwrite input file.
error TS5055: Cannot write file '/Users/joao/Work/vscode/src/vs/workbench/buildfile.js' because it would overwrite input file.

@joaomoreno

Copy link
Copy Markdown
Contributor

Erich Gamma (@egamma) Also... how exactly does src/buildfile.js get checked? It doesn't have a @ts-check marker nor does its corresponding tsconfig.json have checkJS...

@egamma

Copy link
Copy Markdown
MemberAuthor

Erich Gamma (@egamma) As Travis and AppVeyor complain, there's a problem with this. The build folder fails to build:

The issue is that when you enable allowJS in the tsconfig.json, then TypeScript will transpile the JS files. I need to find a way to specifc --noEmit for the .js files.

@egamma

Copy link
Copy Markdown
MemberAuthor

Erich Gamma (@egamma) Also... how exactly does src/buildfile.js get checked? It doesn't have a ts-check (@ts-check) marker nor does its corresponding tsconfig.json have checkJS...

I was surprised as well, but the TS type checker reaches it also when running tsc --noEmit from the build folder. So I assume there is a dependency from a source file in the build folder to src/buildfile.js.

@joaomoreno

Copy link
Copy Markdown
Contributor

@egamma

Erich Gamma (egamma) commented Jan 26, 2018

Copy link
Copy Markdown
MemberAuthor

Here is the issue, when you want to enable checkJS in the tsconfig.json then you also must enable allowJS. Now, when you enable allowJS then TypeScript will compile and emit output for .js files (useful when you want to downlevel compile JS).

I see different options:

  1. add ts-check (@ts-check) to all the .js files we want to have type checked. Downside is that we must remember to add the comment to the .js files. Also it is not clear to me how I can configure the parameters you would configure in the tsconfig.json.

  2. Add the setting "javascript.implicitProjectConfig.checkJs": true as a workspace setting. It seems that this doesn´t affect compilation, but adds many more files to the language server and this might slow down things. Same as for 1. is not clear where you configure the parameters for typescript in this case.

  3. Add two tsconfig.json files to the build folder. One is used by the language service and will be used for checking, one will be used for compiling the project during the build.

I´ll add option 2) to the PR for testing.

@egamma

Copy link
Copy Markdown
MemberAuthor

Hmm, .js files are still compiled so option 2 is not an option...

@egamma

Erich Gamma (egamma) commented Jan 27, 2018

Copy link
Copy Markdown
MemberAuthor

I´ve looked into options 2) and 3). The issue with these options is that the project of the tsconfig.json in the build folder is ignored. This means some typings are not retrieved from the build folder but from the global typings cache. The version of a global typing can differ from the one inside the build folders. This means you get different error messages from Typescript. You can work around them by adding reference path comments that point to the typings in the build folder.

/// <reference path='lib/typings/event-stream.d.ts'/>

Such comments are a pain to maintain and duplicates the project context from the tsconfig.json.

My conclusion is that the checkJS must use the project context defined by tsconfig.json and that Typescript should support that you can enable checkJS without having to enable allowJS. Filed issue microsoft/TypeScript#21435.

@egamma

Copy link
Copy Markdown
MemberAuthor

João Moreno (@joaomoreno) closing in favor of #42731 (this one is messy and mixes different experiments).

@joaomoreno
João Moreno (joaomoreno) deleted the egamma/issue41678 branch February 1, 2018 14:30
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Mar 31, 2020
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

engineeringVS Code - Build / issue tracking / etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@egamma@joaomoreno