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

Current tsconfig naming is problematic w.r.t. VSCode (and other IDEs) #5175

Closed
victornoel opened this issue Mar 2, 2017 · 16 comments
Closed

Comments

@victornoel
Copy link

OS?

archlinux latest

Versions.

@angular/cli: 1.0.0-rc.0
node: 6.10.0
os: linux x64
@angular/common: 4.0.0-rc.1
@angular/compiler: 4.0.0-rc.1
@angular/core: 4.0.0-rc.1
@angular/flex-layout: 2.0.0-rc.1
@angular/forms: 4.0.0-rc.1
@angular/http: 4.0.0-rc.1
@angular/material: 2.0.0-beta.2
@angular/platform-browser: 4.0.0-rc.1
@angular/platform-browser-dynamic: 4.0.0-rc.1
@angular/router: 4.0.0-rc.1
@angular/cli: 1.0.0-rc.0
@angular/compiler-cli: 4.0.0-rc.1

Repro steps.

app created with the cli

Mention any other details that might be useful.

It seems that the choice was made to name tsconfigs as:

  • ./src/tsconfig.app.json
  • ./src/tsconfig.spec.json
  • ./e2e/tsconfig.e2e.json

All extending ./tsconfig.json.

The problem with this is well explained in the following tickets of vscode and typescript:

Basically, vscode is expecting a file named tsconfig.json in a given directory (and multiple can be used, but in different directories).

Currently what happens with angular-cli, is that only the parent tsconfig.json is used by vscode, and thus libraries are missing because only declared in tsconfig.app.json (or one of the others).

It would make sense to at least rename ./e2e/tsconfig.e2e.json to ./e2e/tsconfig.json and ./src/tsconfig.app.json to ./src/tsconfig.json. Or maybe it is possible to tell vscode this but I didn't find how…

And then, I think we are blocked for now concerning the specs files since they are in the same directory as the app files. In the above issues, Microsoft people recommend to have two directory, one for the tests and one for the application.

@victornoel
Copy link
Author

Btw, the same thing happens with other compiler options (such as target!).

@edemirkan
Copy link

edemirkan commented Mar 2, 2017

I can confirm. Missing "target": "es5" in tsconfig.json highlighted my accessors as syntax errors.
screenshot_2017-03-02_09-27-31

@victornoel
Copy link
Author

victornoel commented Mar 3, 2017

Some of the things requested here were fixed in 1.0.0-rc1 via #5046/#5060 (es5 target and dom lib).

This is still open to questions maybe… not sure. @filipesilva, I see you were the one fixing the above issue, should this be closed or do you think the rest of the issue will result on some actions?

@Juansasa
Copy link

Juansasa commented Mar 3, 2017

Still not working for me

@victornoel
Copy link
Author

@nickiewooster you need to update your project :) I think the bug fix is only for new project.

@victornoel
Copy link
Author

(I mean updating you tsconfig.json file)

@Juansasa
Copy link

Juansasa commented Mar 3, 2017

I dont see anyone mention modifying tsconfig.spec.json.
There were no compilation errors but it will be one when updating a test.

screen shot 2017-03-03 at 09 25 36

I have to add the "dom" lib to tsconfig.spec.json as well to fix the issue

@victornoel
Copy link
Author

@nickiewooster sorry, what was your problem originally? Just to be clear :) I thought you had the same one as me with the IDE, not the compilation :)

@victornoel
Copy link
Author

@nickiewooster also, if you need the dom library for your test, feel free to add it, but if I understood well, from the point of view of default newly created angular project, adding the dom lib to the tests is not needed. It should only be present in the application itself.

@Juansasa
Copy link

Juansasa commented Mar 3, 2017

Sorry i was referred here from another thread and got a bit mixed up. You're right the tsconfig modification alone will fix the IDE errors

@filipesilva
Copy link
Contributor

@victornoel it was intentional to only have one tsconfig with the a name that would be recognized by the IDE.

This was our solution to the problem where src/ is actually home to two different 'apps': the Angular app and the unit test app. Each need different configuration and we were specialcasing that before with hacks.

For the sake of consistency we change all tsconfigs to specify their purpose and left the general tsconfig on top.

There were a couple more solutions we considered but at the end of the day settled on this one because it was the simpler to understand.

Failing to include the es5 target and dom lib was just a mistake on my part though.

@victornoel
Copy link
Author

@filipesilva ok, thanks for taking the time to explain!

So I guess we can simply close this issue now that it was fixed in rc1 :)

@Ionaru
Copy link

Ionaru commented Mar 3, 2017

On compiling, I now get the error Cannot find namespace 'NodeJS'
It references an import of import Timer = NodeJS.Timer;
Is this related? The import and typing worked fine before the tsconfig change.

@filipesilva
Copy link
Contributor

@Ionaru you might need to add it to the types array of either app/specs/e2e tsconfig (depending on where you're using it).

@Ionaru
Copy link

Ionaru commented Mar 3, 2017

It worked when I used "types": ["node"], thank you.

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants