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

Properties of type List<T> result in any[] even when T defined in same project #17

Open
idg10 opened this issue May 1, 2017 · 10 comments

Comments

@idg10
Copy link

idg10 commented May 1, 2017

Installed product versions

  • Visual Studio: 2017 Enterprise v15.1 (26403.7)
  • This extension: v1.0.12
  • Windows 10 Creators Update (up to date as of 2017/05/01)

Description

When a property of a class is of type List<T> for some T which is a) defined in the same project, and b) has already had its Generate TypeScript Definitions context menu item checked, the TypeScript property will be of type any[] instead of producing useful static type information.

Steps to recreate

Create a project (e.g., a plain .NET Class Library, although this seems to happen in any project type I've tried, including also classic ASP.NET, ASP.NET Core, and .NET Standard class library projects), and add this in a Child.cs file:

public class Child
{
    public int NumericValue { get; set; }
    public string TextValue { get; set; }
}

Right click and check Generate TypeScript Definitions

Then add this in Parent.cs

public class Parent
{
    public Child Item { get; set; }
    public List<Child> Items { get; set; }
}

Right click and check Generate TypeScript Definitions

Current behavior

The extension produces this TypeScript as Parent.d.ts:

declare module server {
    interface parent {
	    item: {
		    numericValue: number;
		    textValue: string;
	    };
	    items: any[];
    }
}

There are two problems here. The first has already been reported in #7 - the item property is not using the server.child type in the generated Child.d.ts file, and has instead inlined the type definition, so it does at least have the correct shape. That's obviously not the problem I'm reporting here, since there's already an open issue for that, but I mention it because it's hard to miss.

The second problem here, and this is the one I'm raising this issue for, is that the items property has type any[].

Expected behavior

The items property should have type server.child[]. Or, for as long as #7 remains open, it should at least produce an inlined array type with the correct shape.

Root cause

Having single-stepped through this in the debugger, I have found that the problem occurs at https://github.com/madskristensen/TypeScriptDefinitionGenerator/blob/master/src/Generator/IntellisenseParser.cs#L287 in IntellisenseParser.TryToGuessGenericArgument. This code correctly determines the full name of the relevant type, and passes that to projCodeModel.CodeTypeFromFullName. But unfortunately, this returns a CodeType whose InfoLocation returns vsCMInfoLocationExternal even though the type in question is in fact defined in the same project.

This seems like it might be a Visual Studio 2017 bug. (I'm asking a friend on the VSIP program if he can find anything out about this.) Or possibly this is not the best way to obtain a CodeType for a type in the same project.

@idg10
Copy link
Author

idg10 commented May 1, 2017

Fixing this depends on fixing #7 first, because you do not support generating anonymously typed collection properties. (The !isCollection in https://github.com/madskristensen/WebEssentials2015/blob/9a9eaa4455f9b146981252d7b03e45db3adf34ce/EditorExtensions/Misc/CodeGeneration/IntellisenseParser.cs#L255 explicitly rules this out.) I don't know TypeScript well enough to know for certain but perhaps it's not actually possible to define the shape inline for an array type like you can for non-collection members?

Anyway, once #7 is fixed, I have a potential workaround for what appears to be a Visual Studio 2017 issue (the fact that the CodeType you get back from projCodeModel.CodeTypeFromFullName incorrectly reports that the type is external, as mentioned in my initial report). If you change TryToGuessGenericArgument to this:

private static CodeTypeRef TryToGuessGenericArgument(CodeClass rootElement, CodeTypeRef codeTypeRef)
{
    var codeTypeRef2 = codeTypeRef as CodeTypeRef2;
    if (codeTypeRef2 == null || !codeTypeRef2.IsGeneric) return codeTypeRef;

    // There is no way to extract generic parameter as CodeTypeRef or something similar
    // (see http://social.msdn.microsoft.com/Forums/vstudio/en-US/09504bdc-2b81-405a-a2f7-158fb721ee90/envdte-envdte80-codetyperef2-and-generic-types?forum=vsx)
    // but we can make it work at least for some simple case with the following heuristic:
    //  1) get the argument's local name by parsing the type reference's full text
    //  2) if it's a known primitive (i.e. string, int, etc.), return that
    //  3) otherwise, guess that it's a type from the same namespace and same project,
    //     and use the project CodeModel to retrieve it by full name
    //  4) if CodeModel returns null - well, bad luck, don't have any more guesses
    var typeNameAsInCode = codeTypeRef2.AsString.Split('<', '>').ElementAtOrDefault(1) ?? "";
    CodeModel projCodeModel;

    var typeName = TryToGuessFullName(typeNameAsInCode);
    try
    {
        projCodeModel = rootElement.ProjectItem.ContainingProject.CodeModel;

        foreach (ProjectItem projectItem in rootElement.ProjectItem.ContainingProject.ProjectItems)
        {
            if (projectItem.FileCodeModel?.CodeElements != null)
            {
                CodeElement matchingElement = FindMatchingCodeElement(
                    projectItem.FileCodeModel.CodeElements,
                    codeElement => codeElement.IsCodeType && codeElement.FullName == typeName);
                if (matchingElement != null)
                {
                    return projCodeModel.CreateCodeTypeRef(matchingElement);
                }
            }
        }
    }
    catch (COMException)
    {
        projCodeModel = _project.CodeModel;
    }

    var codeType = projCodeModel.CodeTypeFromFullName(TryToGuessFullName(typeNameAsInCode));

    if (codeType != null) return projCodeModel.CreateCodeTypeRef(codeType);
    return codeTypeRef;
}

private static CodeElement FindMatchingCodeElement(CodeElements elements, Func<CodeElement, bool> matches)
{
    foreach (CodeElement child in elements)
    {
        if (matches(child))
        {
            return child;
        }

        if (child.Children != null)
        {
            CodeElement result = FindMatchingCodeElement(child.Children, matches);
            if (result != null)
            {
                return result;
            }
        }
    }

    return null;
}

it works around the problem. The basic idea here is that before we call projCodeModel.CodeTypeFromFullName we first scan through all the source files in the project, looking for a CodeElement representing the type we're looking for. This works because a CodeElement discovered this way always reports an InfoLocation of vsCMInfoLocationProject unlike the ones returned by projCodeModel.CodeTypeFromFullName. This code still falls back to projCodeModel.CodeTypeFromFullName if we can't find the type via the project items (which will be the case when the type is defined in some other project).

I would create a PR for this, but resolving #7 also requires #13 to be addressed, and I can't tell which way you actually want to go with #13 - you could fix it either by reverting to the Web Essentials 2015 file naming convention, or you could update the problematic code to work with the new convention. But since I don't know which of those you'd want, I can't create a suitable PR to fix #7 and #13 both of which need to be fixed to enable this issue to be fixed.

@raboud
Copy link
Contributor

raboud commented May 9, 2017

Current output
#18
declare module server { interface parent { item: server.Child; items: server.Child[]; } }

@raboud
Copy link
Contributor

raboud commented Jun 14, 2017

Please Try Build 1.1.24 to see if your issues are corrected.

@justdmitry
Copy link

VS2017, TypeScriptDefinitionGenerator v1.1.24

Original C# code:

public class BackgroundProcessingInfo
{
    public List<NameProgress> ActiveJobs { get; set; }

    public class NameProgress
    {
        public string Name { get; set; }

        public byte Progress { get; set; }
    }
}

Generated TS:

declare module server {
	interface nameProgress {
		name: string;
		progress: any;
	}
	interface backgroundProcessingInfo {
		activeJobs: any[];
	}
}

@idg10
Copy link
Author

idg10 commented Jul 11, 2017

As far as I can tell, with Visual Studio 2017 Update 2 build 26430.15, the code model is no longer reporting the wrong location - the CodeType returned by CodeTypeFromFullName now reports an InfoLocation of vsCMInfoLocationProject.

So the workaround I proposed no longer appears to be necessary. Now that VS is behaving correctly, and you've also fixed #7 and #13 I'm no longer seeing this problem.

However, I think @justdmitry has identified a new issue. I find that if I use his example C#, I have to save the C# file an extra time (triggering a 2nd code generation) to get the correct output. The first time, I see the same TS he reports. But I make a trivial change to the C# file and re-save it, I then get the expected TS:

declare module server {
    interface nameProgress {
	    name: string;
	    progress: any;
    }
    interface backgroundProcessingInfo {
	    activeJobs: server.nameProgress[];
    }
}

Perhaps there is an ordering problem with nested types?

Update 2017/7/31: see my later comment below - I spoke too soon. This is not in fact working for me in my real project.

@mbreaton
Copy link
Contributor

mbreaton commented Jul 23, 2017

This is still an issue for me. I am using an asp.net core project (where its content is implicit). I think the problem arises in the IntellisenseParser.GetType (line 292) where isCollection is true causing the expression to return false (and thus never setting the result.Shape)

if (!isPrimitive && codeClass != null && !traversedTypes.Contains(effectiveTypeRef.CodeType.FullName) && !isCollection)
{
    ... set result.Shape
}

I refactored it to remove the isCollection check

if (!isPrimitive && codeClass != null && !traversedTypes.Contains(effectiveTypeRef.CodeType.FullName))

which seems to resolve the issue I'm seeing on my side, but I'm unsure as to the isCollection check's purpose (perhaps more recent architectural changes make this obsolete?) and the ramifications of its removal.

@StefanKern
Copy link

@mbreaton will this fix be in a new release soon? I'm getting the same problem, that all my types are any. My code is identical to @justdmitry

@idg10
Copy link
Author

idg10 commented Jul 31, 2017

I've realised I was premature in saying that I'm no longer seeing these problems. It turns out that in the simple class library project (using the new VS15-style csproj format) I no longer see any with the v1.1.24 of the extension and the latest VS update (15.2 (26430.16)). However, with my original project I still in fact see the issue. And with some more experimentation, it seems that if I create an ASP.NET Core project targetting the full .NET Framework (which is the scenario in which I first ran into this) I'm still getting any[] for lists where you would expect a typed array.

@mdelgadov
Copy link

I checked on this and I think it's not a bug, Typescript tries to analyze the list and deduct the type, if it can't, then creates a any[].
Also, if a foreign relation can't be analyzed, it becomes an object.

To solve this, I had to create the dependencies first and then the object, for example, create currency first (no dependency, just a number, a symbol, etc) and then create money.d.ts (that has currency on it).

So, it works but needs to have all dependencies solved first.

@idg10
Copy link
Author

idg10 commented Aug 7, 2017

I'm not quite sure what you mean, @mdelgadov when you say "Typescript tries to analyze the list" - the lists in this case are all defined in C#, so how can TypeScript be trying to analyze it? The point of this extension is to extract the type definitions from C# source code and then generate TypeScript code from that. So the problem here isn't going to be anything to do with what TypeScript is doing - by the time TypeScript gets a look in, this extension has already finished its work.

Moreover, this used to work with the VS2015 version of this feature (back when it was a feature of WebEssentials instead of a separate extension). So to regard this as not a bug doesn't seem appropriate, since it's a regression from how the previous version worked.

(Also, TypeWriter - https://github.com/frhagn/Typewriter/releases - is able to generate suitable TypeScript definitions from C# types that involve List<T>. I've ended up migrating to TypeWriter now, partly due to this problem, but also because TypeWriter makes it practical to generate type definitions from classes defined in other projects.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants