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

TypeScript Language Service implicit referencing functionality breaks in Visual Studio... #673

Closed
johnnyreilly opened this Issue Sep 13, 2014 · 15 comments

Comments

Projects
None yet
3 participants
@johnnyreilly
Copy link

johnnyreilly commented Sep 13, 2014

...with a second TypeScript project in a solution that uses /// <reference path= comments. I found this issue as I was migrating a Jasmine unit test project to TypeScript. You can see the full details in this blog post.

I had a web project that was written using TypeScript and a separate unit test project in the solution which contained tests for the web project. It was written using JavaScript. I migrated the unit test project to TypeScript through the use of /// <reference path= comments and added typings. Doing this, somehow broke the implicit referencing behaviour built into Visual Studio for the original web / TypeScript project. If I pull out the /// <reference path= comments from the top of the migrated test file that I've converted then it's business as usual - the TypeScript Language Service lives once more.

I suspect that if I added /// <reference path= comments throughout the web project the TypeScript Language Service would be just fine. But I rather like the implicit referencing functionality so I'm not inclined to do that. My guess is that this is a bug rather than intended functionality so I thought I should raise it here.

For the ease of anyone trying to replicate this issue I have created the "Jasmine_tests_migrated_to_TypeScript" branch of Proverb which demonstrates the code in the problematic state. If you open this up in Visual Studio 2013 (Update 3) then should see the same issue I do.

@mhegazy

This comment has been minimized.

Copy link
Contributor

mhegazy commented Sep 17, 2014

@johnnyreilly I am having hard time understanding the issue. I did download your sources, and I do see see "cannot find name" errors, which seem legitimate to me. so I will need your help to understand what is the issue here.

The easiest way to see how the language service understands your project structure, is to enable virtual projects, to do that, go to tools\options\text editor\TypeScript\Project and check "Display Virtual Projects when a Solution is loaded".

this should show you something like:

capture

I see you have two TypeScript enabled projects, "Proverb.Web" and "PRoverb.Web.Tests.JavaScript", the first one seems fine, the second is missing some files, that I expect you meant to add.. e.g. logger.ts, I see it in one, but not the other, and looks like common.ts is using it, but it can not find it.

The way a context is created, is first collecting all files references in the project file with "TypeScriptCompile" build action, and then recursively walking all the /// references. the resulting closure is your project context, that is the same on the command line as well.

I would appreciate it if you tell me what I need to look at, like which like I should remove to get to an inconsistent state, and what is the expected correct state that I should be looking for.

@johnnyreilly

This comment has been minimized.

Copy link

johnnyreilly commented Sep 17, 2014

Hi @mhegazy,

Thanks for the detailed response. I'll look into this and report back.

@johnnyreilly

This comment has been minimized.

Copy link

johnnyreilly commented Sep 17, 2014

Hi @mhegazy,

Following your comments on missing references I went through and added all the missing references to the sageDetail.ts test file:

/// <reference path="../../../proverb.web/scripts/typings/angularjs/angular-route.d.ts" />
/// <reference path="../../../proverb.web/scripts/typings/toastr/toastr.d.ts" />
/// <reference path="../../../proverb.web/scripts/typings/underscore/underscore.d.ts" />
/// <reference path="../../../proverb.web/app/common/logger.ts" />
/// <reference path="../../../proverb.web/app/services/repositories.ts" />
/// <reference path="../../../proverb.web/app/services/repository.saying.ts" />
/// <reference path="../../../proverb.web/app/app.ts" />
/// <reference path="../../../proverb.web/app/config.route.ts" />

As I added each missing reference to the the sageDetail.ts test file in the Test project the related "Could not find symbol..." errors would disappear from the TypeScript Language Service inside the Web project. I've update the branch in Proverb with all the missing references.

If you want to replicate the issue then you could pick the latest commit in Proverb which will give you the app with no issues. To introduce the problem go to the sageDetail.ts test file in the Test project and remove this reference:

/// <reference path="../../../proverb.web/app/config.route.ts" />

This will result in the following "Could not find symbol..." error in app.ts in the Web project:

couldnotfindsymbol

So the problem is essentially resolved but I would say the tooling experience is a little confusing. As I understand it, the following holds true:

  • Individual TypeScript projects stand alone - problems within 1 TypeScript project will not affect the compilation of another TypeScript project.
  • The same does not hold true for the TypeScript Language Service. It appears to share context across all TypeScript projects that exist in a project. The upshot of this is that if you have missing references in one project, it may manifest itself in the form of "Could not find symbol..." errors in the other projects.

Is my understanding correct?

This also prompts me to ask, how did you spot the missing references issue? I believe I was able to compile the sageDetail.ts test file without the missing references. I didn't notice any prompts in Visual Studio to tell me there were missing references. How did you pick up on it?

It may be that this issue can be closed. However, I think it would be good to improve the tooling experience to make these sorts of issues more obvious to the user so they don't end up logging issues (as I have). What do you think?

@mhegazy

This comment has been minimized.

Copy link
Contributor

mhegazy commented Sep 17, 2014

I could not compile them locally. I was missing some nugget packages, and did not attempt to download them to be frank.

In general. if you are using the standard targets, standard properties, and all your files are in TypeScriptCompile tags, and you seem to do, your build behavior should be identical to your Language Service behavior; if not, that would be a bug

As for contexts, and how I spotted the missing references, the project view in the solution explorer does not match that of the language service once you use /// references, as you are pulling more files. We use the "Virtual Project" view to diagnose these issues. if you look at my screenshot, you will see a node in the solution explorer: "TypeScript Virtual Projects" and underneath that there are two virtual projects nested, one for each actual project that has typescript files.

To enable this view, go to tools\options\text editor\TypeScript\Project and check "Display Virtual Projects when a Solution is loaded".

Each project is considered a separate context, no bleeding shall occur, each has its own type checker, errors, and even different snapshot of the library.

Each context is built starting from the TypeScriptCompile files in project, then following the transitive closure of the /// references.

So accordingly, if you reference one file through ///references it will be pulled in the context, if the file containing the /// references is not in the context, then neither will be the referenced files.

Moreover, the error list has a "Project" column, when I looked, all the errors were in one project (Proverb.Web.TEsts.JavaScript) and not the other; which is the one missing your references.

My recommendation is not to mix project contexts and /// references, for two reasons: first it gets really hard to understand what is in and what is out, as you have experienced personally; and second you do not get incremental build working correctly, as incremental build only consider the files in the project to know if the project is up-to-date, and does not look at /// references, so you could change a /// referenced file, and build, and get no errors, as the build was skipped, because none of the files in the project changed.

Alternatively, you can add shared files "as links" in your test project.

capture

Hope that answers your questions.

@johnnyreilly

This comment has been minimized.

Copy link

johnnyreilly commented Sep 18, 2014

Hi @mhegazy,

I'm using the TypeScript Virtual Projects support as you advise (actually I started using it when you initially mentioned it - I just didn't say!). In response to your comments:

Alternatively, you can add shared files "as links" in your test project.

This sounds like it is worth trying. My intention is try and keep my TypeScript tests separate from the main project for 2 reasons:

  1. I find the separation of tests and application logic quite helpful - I'm quite keen on modularity and this approach allows for smaller projects with more distinct purposes. Also it fits with my existing .NET test projects - they are separate from the main codebase as well and it makes sense to continue the pattern.
  2. This means that the tests are not shipped with the app when I release - or at least I don't have to take extra steps to remove them from the build as part of the release process

I used the /// <reference approach because I thought it was the only game in town. Given what you've said maybe the add shared files "as links" approach would make more sense.

In general. if you are using the standard targets, standard properties, and all your files are in TypeScriptCompile tags, and you seem to do, your build behavior should be identical to your Language Service behavior; if not, that would be a bug

As far as I know I am using the standard targets, standard properties, and all files are in TypeScriptCompile tags. So it's possible there is a bug. Language Service and build behaviour do appear to differ.

Given what you've said about project contexts above I'm wondering how the Language Service fits with this. As you move from one file in one project to a different file in a different project is the TypeScript Language service supposed to be swapping contexts? Or does it have it's own context? In which case - how is that constructed? Perhaps that would explain the apparent "bleeding" I appear to have experienced.

What is the best way to determine if this is actually a bug or not would you say?

@NoelAbrahams

This comment has been minimized.

Copy link

NoelAbrahams commented Sep 18, 2014

@johnnyreilly,

Why not do the following?

  • Compile Proverb.Web with --declarations and the option for combining output into a single file.
    This should create a Proverb.Web.d.ts in your output directory.
  • In Proverb.Web.Tests.JavaScript add a <reference> to this file.
  • Right-click Proverb.Web.Tests.JavaScript select "Build Dependencies" > "Project Dependencies" and add a reference to Proverb.Web.

I don't think directly referencing TypeScript source files is a good idea, because it causes the file to be rebuilt every time the dependant project is compiled.

@mhegazy, the virtual projects set up is interesting. Perhaps there should be more documentation on how to use it in order to debug issues. I have experienced the problem mentioned by @johnnyreilly, namely:

...if you have missing references in one project, it may manifest itself in the form of "Could not find symbol..." errors in the other projects.

@johnnyreilly

This comment has been minimized.

Copy link

johnnyreilly commented Sep 18, 2014

Interesting thoughts @NoelAbrahams - glad to know I'm not alone in experiencing this!

@mhegazy

This comment has been minimized.

Copy link
Contributor

mhegazy commented Sep 18, 2014

I have had that on my todo list for a while now, along with _references.ts, which is another way to order your files in a context. I will use this bug to add more documentation. thanks for the nudge :)

I do like @NoelAbrahams suggestion of using a .d.ts as an interface. that would be the "right" way to do it once Typescript has tooling support for Project to Project references. if you can live the limited tooling support for this scenario, I think this is the cleanest and best alternative.

@johnnyreilly, distilling it down to a smaller sample would be greatly appreciated, I can take it from there, if you can point me to the steps I need to follow to reach the inconsistent state.

@mhegazy mhegazy added this to the TypeScript 1.3 milestone Sep 18, 2014

@mhegazy mhegazy self-assigned this Sep 18, 2014

johnnyreilly added a commit to johnnyreilly/Proverb that referenced this issue Sep 19, 2014

@johnnyreilly

This comment has been minimized.

Copy link

johnnyreilly commented Sep 19, 2014

Agreed a cut down version would be better @mhegazy. I've created another branch of Proverb which starts this for you by stripping out most of the projects. It's in working state as is.

To get to the inconsistent state go to the sageDetail.ts test file in the Test project and remove this reference:

/// <reference path="../../../proverb.web/app/config.route.ts" />

This will result in the following "Could not find symbol..." error in app.ts in the Web project. (The same error mentioned earlier in the conversation.) Hopefully that's enough to allow you to repro - let me know if there's anything else required.

@johnnyreilly

This comment has been minimized.

Copy link

johnnyreilly commented Sep 23, 2014

I've updated my original blog post on this to clarify the 2 solutions that came out of this. You can see it here.

@NoelAbrahams I've included your contribution and attributed it to you - hope that's okay? By the way - looks like you're a London chap too?

@NoelAbrahams

This comment has been minimized.

Copy link

NoelAbrahams commented Sep 23, 2014

@johnnyreilly, interesting blog. No problem with the mention. Yes, from London - might run into you at Tesco!

@johnnyreilly

This comment has been minimized.

Copy link

johnnyreilly commented Sep 23, 2014

Every little helps @NoelAbrahams 😄

@mhegazy

This comment has been minimized.

Copy link
Contributor

mhegazy commented Apr 21, 2015

@johnnyreilly is this still an issue?

@johnnyreilly

This comment has been minimized.

Copy link

johnnyreilly commented Apr 21, 2015

Not really @mhegazy - essentially tsconfig.json removes the need for reference comments. (Well I believe there's a few edge cases where they are handy but none that affect me.)

So I think close the issue - and much ♥ for tsconfig.json

@mhegazy

This comment has been minimized.

Copy link
Contributor

mhegazy commented Apr 21, 2015

thanks!

@mhegazy mhegazy closed this Apr 21, 2015

@Microsoft Microsoft locked and limited conversation to collaborators Jun 18, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.