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

TS2134 build error for 2 isolated TS files (shouldn't conflict) #1516

Closed
jt000 opened this Issue Dec 16, 2014 · 19 comments

Comments

Projects
None yet
4 participants
@jt000
Copy link

jt000 commented Dec 16, 2014

This is related to DefinitelyTyped/DefinitelyTyped#2734
See repo at https://github.com/jt000/RepoForTypescriptBuildIssue

The issue is that in a single project I have two TS files that are for separate scenarios, which don't overlap in functionality (common for nodejs projects that have client, server, and test code in one project). Since there is a commonly named variable with different types the typescript compiler throws the TS2134 build error. However, since neither TS files share common references this should be allowed.

@jt000

This comment has been minimized.

Copy link

jt000 commented Dec 17, 2014

I've noticed that this only occurs when TSC is called with all files (as the typescript target in VS does). If it is called once for each file, then it behaves as expected

C:\git\RepoForTypescriptBuildIssue>tsc server.ts

C:\git\RepoForTypescriptBuildIssue>tsc client.ts

C:\git\RepoForTypescriptBuildIssue>tsc server.ts client.ts
C:\git\RepoForTypescriptBuildIssue\clientHelper.d.ts(1,13): error TS2134: Subsequent variable declarations must have the same type.  Variable 'pi' must be of ty
pe 'number', but here has type 'string'.
@mhegazy

This comment has been minimized.

Copy link
Contributor

mhegazy commented Dec 17, 2014

All files passed to the compiler in one go are considered as a unit. Declarations from one file are visible in all other files.

what you want is two projects (two compilation units), one for your server and one for your client. The shared files can live in one project and imported using /// references in the other.

Hope that helps.

@mhegazy mhegazy added the Question label Dec 17, 2014

@jt000

This comment has been minimized.

Copy link

jt000 commented Dec 17, 2014

Thanks @mhegazy

Multiple projects aren't very convenient for node projects since you'd have to share one package.json (which breaks the nodejs tool projects in VS) or have it nested in a sub-folder, which breaks some git-deployments tools that expect it at the root (i.e. Azure Websites).

Since each TS file can produce its own JS, then it seems like each one should be treated separately from each other. Assuming all TS files are related also assumes that one is going to load all JS files together at runtime (in which case, why not just build a single JS file).

The only reason I can see figure for lumping them all together when each TS builds its own JS file is to prevent people from including a bunch of <reference> tags (correct me if there's another benefit to this). In the Module case (AMD or CommonJS), then there will be an import to show references for everything but the d.ts files, so this scenario wouldn't impact this scenario as much.

All this said, I realize this would be a breaking change, so I'm just asking for an "opt-out" of this functionality, which could probably be handled in the targets file or the typescript.tasks.dll rather than in the tsc.exe (though if done in tsc.exe you could do some optimizations if you were so inclined...)

@jt000

This comment has been minimized.

Copy link

jt000 commented Dec 17, 2014

Passing all the TS files together to the compiler also seems to cause an issue with using /// <reference no-default-lib='true' />. If you included this on a single TS file, then it prevents all files from using the default-lib when a user may have intended this to only apply to the file that it was added to.

@mhegazy

This comment has been minimized.

Copy link
Contributor

mhegazy commented Dec 17, 2014

The compiler needs to know a "context" to be able to tell you that you are defining a class or using an undefined variable. All files passed on the command line in a call to tsc are considered part of the same context. That has the side effect of complicating client server app building; but without that the compiler has no other way to identify intent. Like did you really mean to define a function twice because you have two output files that are used independently or because you just made a mistake.

One way is to use external modules with no global scope code. So your server is a node module, your client is another module and shared code is another one that both depend on.

You can also use internal modules to identify different namespaces; so module common, module client and module server. This way no redefinition of the same function or variable.

@jt000

This comment has been minimized.

Copy link

jt000 commented Dec 17, 2014

Thanks again @mhegazy

All files passed on the command line in a call to tsc are considered part of the same context. That has the side effect of complicating client server app building; but without that the compiler has no other way to identify intent. Like did you really mean to define a function twice because you have two output files that are used independently or because you just made a mistake.

I get what you're saying, but I think the Visual Studio typescript build is catering to a specific scenario of users. Ones that have files a.ts, b.ts, & c.ts which compile to a.js, b.js, & c.js and they are all used together in html like:

<script src="a.js" />
<script src="b.js" />
<script src="c.js" />

For many scenarios it doesn't work that way. If one wanted to use all the TS files together, then they probably would've just used the -out parameter to compile to a single file. You are correct that without context it is possible for the user to shoot themselves in the foot, but I think the VS typescript build is assuming that the context is equal to a project & for individual JS files and external modules that just isn't the case. Rarely (if ever) will a website load up all the external modules or JS files all together. Often sites have a specific JS file per page, which could feasibly have the same function twice...

For reference, users that use Grunt or Gulp execute tsc on single files at a time (via the grunt-ts extension) & are use to putting <reference /> at the top of their files to use classes in other TS files. Many times they do create TS files that contains multiple references, but these can be catered to different groupings of TS files with multiple grunt tasks (i.e. all TS files in the /server folder uses <reference path="/server/references.ts" /> while all TS files in the /client folder use <reference path="/client/references.ts" />). For example: https://www.youtube.com/watch?v=Km0DpfX5ZxM

I am not sure I understand your two suggestions. In the linked bug DefinitelyTyped/DefinitelyTyped#2734 we have angular-protractor which exposes a $ var and jquery which also exposes a $ var of a different type. Since they will never be used in the same context, then there should be a way to get this to work without an error message when they are in the same Visual Studio project. Will one of your suggestions fix this issue?

@jasond-s

This comment has been minimized.

Copy link

jasond-s commented Feb 17, 2015

+1 would be nice if VS ext could understand the references paths.

@mhegazy

This comment has been minimized.

Copy link
Contributor

mhegazy commented Feb 20, 2015

@jt000 sorry the delayed response, this fell off my radar during the holidays.

we have angular-protractor which exposes a $ var and jquery which also exposes a $ var of a different type. Since they will never be used in the same context, then there should be a way to get this to work without an error message when they are in the same Visual Studio project.

If they are never used together in a single application then they should not be passed to the compiler in a single compilation. you can still share your common files using /// references. this way if you ever mistakenly use $ from jQuery in the project that does not use jQuery the compiler will be able to alert you to that instead of silently failing.

so my suggestion is simply use two projects. for instance, if you have two projects (webpage1, webpage2) where both use jQuery.d.ts, and another not related one that does not.

  • jQuery.d.ts
  • common.ts
  • webpage1.ts
  • webpage2.ts
  • anotherNotRelatedPage.ts

in webpage1.ts, webPage2.ts you add /// and ///

and call the compiler using:

tsc webpage1.ts webpage2.ts

and in anotherNotRelatedPage.ts add a reference to ///

and call the compiler using:

tsc anotherNotRelatedPage.ts

this way you have two "contexts", and you are still sharing common.ts

you can do that in VS as well, just create two projects.

hope that helps.

@jt000

This comment has been minimized.

Copy link

jt000 commented Feb 20, 2015

Thanks @mhegazy for the reply.

Applications make sense in a desktop environment, but when working with javascript what constitutes an "application"? Each Node.js project? Each page that's hosted in a site? Each different entry-point for "npm start x"? Javascript files are extremely flexible in how they can be loaded & run and I don't think its fair to say that there is even a single application even for the same JS file.

What you suggest for having multiple projects is a fair workaround, but I don't think it would be what you'd want to suggest as a de facto process for creating typescript files. When using CommonJS or Require each TS file should theoretically have their own context, which would suggest they need to be in their own project. This would get unmanageable very quickly... Or you could create them all in the same context until you ran into a conflict, then pull it out into its own project, but this isn't really the best user experience. It would be much better to have a solution for this built into VS build for typescript.

The way I've resolved this was to remove VS from the equation entirely and instead used Gulp within WebStorm, so that I can define what my own contexts are without creating new projects. This allows me to use multiple tsc.d.ts files within the same "project" and even recompile multiple flavors of my end JS file. (i.e. compile A.ts, B.ts, D.ts into "DPage.js", compile A.ts, B.ts, C.ts into "CPage.js"). I would be surprised if there wasn't a way to design the VS build for typescript to do something similar without using multiple VS projects (i.e. using VS project item properties, maybe).

As a ex-Microsoft employee, I'd love to help you understand my scenario so you can improve the VS building of TS files and ensure more people in the future don't migrate to WebStorm or other IDEs for their Typescript development environment... This isn't blocking for me, but I would like to get back to supporting Visual Studio (it's such a great IDE). (Feel free to send me an email if you'd prefer to speak offline)

@NoelAbrahams

This comment has been minimized.

Copy link

NoelAbrahams commented Feb 20, 2015

What you suggest for having multiple projects is a fair workaround, but I don't think it would be what you'd want to suggest as a de facto process for creating typescript files

Why not? The idea of having a project context for individual files (or tiny collections of them) seems rather strange and will not scale at all.

Or you could create them all in the same context until you ran into a conflict, then pull it out into its own project, but this isn't really the best user experience.

That's a somewhat ad-hoc approach. Each project is a logical and physical container for related files. Their composition should be planned out in advance.

@mhegazy

This comment has been minimized.

Copy link
Contributor

mhegazy commented Feb 21, 2015

So external modules, whether commonJS or require (AMD) are nice in the sense that they declare dependencies and do not pollute the global namespace. So these can all be in the same project and the compiler would know what dependencies they need and they will not cause conflicting definitions, etc.. and the compiler will emit each to its file. this this is sort of a happy scenario. i would just lump all node modules in one project for instance, unless you are building them with different configurations (e.g. --noImplictAny for some but not others).

The problem arises from JS files that would be included in an html page in script tags, these can affect each other, and the compiler here has two options, either 1. do not bother when it sees a conflicting definition of the same variable or a use of an undefined global, or 2. complain. I think the second option is generally more helpful.

I understand that adding a project file for every entry point may be a lot of work. for this we have added support for tsconfig (#1667), which we plan to support in VS as well. Using tsconfig, you would divide your code base in folders for instance, and you would add a tsconfig.json to each folder representing a website for instance (i.e. all scripts that are expected to live together) including shared files. the tsconfig file can be empty, no need to list your files unlike a project file, it implicitly include all files from the folder. In VS you just open a file, and VS will figure out which tsconfig file to load and what other files are part of this context and will load them for you.

I think this should work well in your scenario with no project creation/management, then you can wire your gulp/grunt build to use tsconfig and you will be all set.

I would be happy to get more feedback about your specific scenario and we definitely appreciate your input.

@jt000

This comment has been minimized.

Copy link

jt000 commented Feb 21, 2015

Thanks @mhegazy for taking the time to understand the scenario and for the write-up. I was unaware of the tsconfig #1667 feature you're adding. Once there is support for that in VS, then I think this will greatly help resolve DefinitelyTyped/DefinitelyTyped#2734 by making it easier to have both client & server TS files (or test & production TS files) in the same project which is very common in node.js projects.

Is VS support of #1667 planned?

@mhegazy

This comment has been minimized.

Copy link
Contributor

mhegazy commented Feb 21, 2015

Yes. Hopefully in the next release.

@NoelAbrahams

This comment has been minimized.

Copy link

NoelAbrahams commented Feb 22, 2015

@mhegazy,

the tsconfig file can be empty, no need to list your files unlike a project file, it implicitly include all files from the folder

I believe this is true, provided no files have been added to the files property. Can the logic be extended to the following:

tsconfig implicitly includes all files from the folder, provided no files _from the active directory_ (or subdirectories) have been specified. If files from the active directory or subdirectories have been added then only those files will be included. _External references_ are not included in the calculation.

So, it should be possible to say: "Include external reference ..\foo\bar\baz.ts pus all files from the active directory and subdirectories."

This would cater for having code in multiple projects, with references between them.

Without this proviso it will be difficult to convert our current source tree to use tsconfig.

@mhegazy

This comment has been minimized.

Copy link
Contributor

mhegazy commented Feb 22, 2015

@NoelAbrahams can you provide more details and why combing tsconfig with no files is not sufficient for you (combined with ///references to externals)?
It sounds like you want glob support in files, something like ./*/.ts, is that correct?

@jt000

This comment has been minimized.

Copy link

jt000 commented Feb 22, 2015

Are these comments for #1667 or do they relate to this issue somehow?

@NoelAbrahams

This comment has been minimized.

Copy link

NoelAbrahams commented Feb 22, 2015

@mhegazy,

I want to add external references, but not have to list all the files in the current project.

(@JT100, it's probably more related to the other issue - sorry for mucking up your issue!)

@mhegazy

This comment has been minimized.

Copy link
Contributor

mhegazy commented Feb 22, 2015

@NoelAbrahams why not add a /// reference in one of your files?

@NoelAbrahams

This comment has been minimized.

Copy link

NoelAbrahams commented Feb 22, 2015

We're hoping to get rid of ///reference altogether. tsconfig is a nice solution. Just shouldn't create more work.

@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.