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

Partial C# classes result in multiple conflicting TS classes #127

Closed
KeithHenry opened this issue Jul 27, 2016 · 14 comments
Closed

Partial C# classes result in multiple conflicting TS classes #127

KeithHenry opened this issue Jul 27, 2016 · 14 comments

Comments

@KeithHenry
Copy link

KeithHenry commented Jul 27, 2016

This is fairly simple to replicate:

Dummy.a.cs
public partial class Dummy { public string A {get; set;} ... }

Dummy.b.cs
public partial class Dummy { public int B {get; set;} ... }

dummy.tst

$Classes(Dummy)[
    export class $Name {$Properties[
        public $name: $Type;]
    }]

Output:

Dummy.a.ts

export class Dummy { 
    public a: string;  
    public b: number;
}

Dummy.b.ts

export class Dummy { 
    public a: string;  
    public b: number;
}

This doesn't compile in TypeScript due to the duplicates. It should either output a single Dummy.ts file or include something in the template for later files to extend (rather than re-define) the class.

The $Classes method should iterate each class, not each file that holds part of a class.

@ronbrogan
Copy link

I ran into this one today, seems to be a bit of a tough problem to solve. As it is now, Typewriter renders each file, so if a file has two classes in it, it will render those two classes into one .ts file. This reflects your code structure and meets expectations. It appears as though each partial class is presented as the full implementation, which makes sense. Partials are only really a way of segregating physical files of code. So, Typewriter sees that each file has a class in it, and it generates an output for that file, each containing the same class.

As it stands, there is a 1 to 1 mapping of input file to output file. To change that structure for partial classes (that certain people regard as a 'bad feature') may not be within scope of the project.

The problem with partial classes, is that there is no relationship between the partial declarations. No given declaration is the 'parent' with others extending that parent. As a consequence, we cannot know which file the partial class should live in, if we wanted to omit emitting the other declaration(s).

Any thoughts?

@KeithHenry
Copy link
Author

Partial classes can be misused, certainly, but what they're intended do in C# is very similar to what TypeWriter is doing.

The commonest place most devs will have seen them is in desktop WinForms (the VS UI has a designer for the form and that gets output as a partial) or Linq DBML files (the record class is created as a partial so that developers can extend it).

Most VS CustomTools generates partial classes. Before partials VS used to autogenerate code in blocks within otherwise user-edited files - you could get nightmare merge errors, the autogeneration was incredibly fragile and extending it (you can now write your own CustomTool extensions) was really hard to do. The certain people that regard them as a 'bad feature' have no idea how VS works - I recommend that they crack out an old copy of VS2003 and build a large WinForms project in .Net 1 to see what it used to be like shudders ;-)

TypeWriter gives us a way to describe a .Net class in TypeScript for our client-side code to access, it's essentially similar to what CustomTool gives us. For instance:

  • FooBar.dbml - describe the DB [Foo] and [Bar] tables with an IDE designer.
    • FooBar.cs - auto generated partial classes rewritten every time FooBar.dbml is changed.
  • Foo.cs - manually edited partial class for Foo records
  • Bar.cs - manually edited partial class for Bar records
  • FooBar.tst - TypeWriter template
    • Foo.model.ts - auto generated TS client-side models for [Foo]
    • Bar.model.ts - auto generated TS client-side models for [Bar]

We're not using Linq, but our pattern looks something like that. Partials give us a clean way to sync the C# to the DB, and TypeWriter could give us a clean way to sync the C# to the TS.

I think there needs to be a solution to this problem.

1 to 1 mapping of .cs files to .ts files is nice, but as C# doesn't have a 1 to 1 mapping of classes to .cs files I think it's going to have to have some flexibility.

Maybe support 2 modes - either 1 to 1 files (with no partial support) or 1 to 1 with classes (but maybe only regenerate on build).

Alternatively group the entire class in the first partial it comes across - it doesn't really matter whether FooBar.model.ts gets generated or Foo.model.ts gets generated, just so long as we don't get both.

@aluanhaddad
Copy link
Contributor

TypeScript does not support partial classes, but it supports interface merging. Most of the time I find an interface is the better solution, especially for code generation of models. You can also merge a classes with several interfaces. This becomes more complicated when using external modules, but it is fully supported by TypeScript.

If using TypeScript namespaces, consider:
dummy.a.ts

namespace Models {
  export interface Dummy {
    a: A;
    b: B;
  }
}

dummy.b.ts

namespace Models {
  export interface Dummy {
    a: A;
    b: B;
  }
}

No conflicts. Declarations are merged.

If you are generating external modules, there are equally effective but slightly more complex strategies. Let me know if these work for you.

@KeithHenry
Copy link
Author

Yes, I have already used a pattern like that for simple model classes where the actual implementation is not important and I just want to ensure that TS and C# code reference the same properties.

I start the files with declare namespace and change the output to end with .d.ts extensions as otherwise you get an empty .js file on transpile.

Having two identical files is clunky and a workaround at best, even if they compile ok.

It is a problem the implementation is important, which unfortunately is most of the time. That isn't really a solution.

@aluanhaddad
Copy link
Contributor

I see. I always use external modules so my workflow is a bit different. As long as all of the behavior is in one of the partial class's constituent files you can mix it up like this.

// partial-a.ts
namespace Models {
    export class SomeModel {
        validate() {
            if (this.name.toLowerCase() !== 'aluan' || this.age < 0) {
                throw Error('Invalid fields')
            }
        }
    }
}


// partial-b.ts
namespace Models {
    export interface SomeModel {
        name: string;
        age: number
    }
}

@KeithHenry
Copy link
Author

KeithHenry commented Sep 20, 2016

I could write TS like that, and we already do extend some of the interfaces in that way, but not using TypeWriter - there's no way to generate your validate method; both partials get all the members.

This is kind of a blocking issue for TypeWriter as a lot of the teams that get value from auto-generating their TS code are highly likely to be using similar tools to auto-generate C#, and that almost invariably means partials.

@aluanhaddad
Copy link
Contributor

aluanhaddad commented Oct 11, 2016

I had a thought about this recently. What if you used Settings.OutputFilenameFactory to ensure that each partial class file received the same name. Then the generation of the second would overwrite the first and that would be OK.

This doesn't work because the FileName get output with (1), (2) suffixed before I can intercept them.

@emilsjog
Copy link

Any updates on this? Or a workaround? To solve this we do comment out the ...(1).ts file, not a fun way to go.
We are using typewriter and encounters this issue almost on a daily basis as we change are model quite often.

@avantidesai
Copy link

bumping for a solution ..... im stuck too !!

@frhagn
Copy link
Owner

frhagn commented Dec 28, 2016

Hi guys,
I've just released a new version with improved support for partial types (1.10.0).
The solution is very similar to @KeithHenry's suggestion using different modes that can be configured in the template constructor.

${
    Template(Settings settings)
    {
        settings.PartialRenderingMode = PartialRenderingMode.Combined;
    }
}
...

PartialRenderingMode.Partial:
Partial types are rendered as defined in the c# source containing only the parts defined in each file. This is the default rendering mode.

PartialRenderingMode.Combined:
Partial type definitions are combined to a single type acting as if the full type was defined only once in the c# source (using the filename of the first file containing a part of the type).

PartialRenderingMode.Legacy:
A combined type definition are rendered for each file containing a partial definition.

Please try it out and let me know if it solves your issues :)

@emilsjog
Copy link

Hi,

PartialRenderingMode.Combined did the work for us, perfect!.
We tried also PartialRenderingMode.Partial but got the same error as before with "duplicate identifiers".

Thanks a lot for this!

@Phil-R-Cole
Copy link

Hi, the instigator of this thread, @KeithHenry, has moved onto new pastures. I am working on the same code-base with our products and I can confirm that employing the PartialRenderingMode.Combined setting seems to have resolved the issues we were facing.
So thanks very much for this enhancement.

@petrosmm
Copy link

I tried PartialRenderingMode.Combined and for some reason, it is not working. Does anyone else have this issue?

@petrosmm
Copy link

Hello all,

I absolutely love the typewriter concept, however, some of my files (one for example) have public partial class ApplicationUser {} twice in the same file reporting a duplicate class declaration. I cannnot seem to figure out what I am doing wrong. The template I am using is below.

The output reports as

module App {
    export class ApplicationUser { 
      // all props
    }

    export class ApplicationUser { 
      // all props
    }
}
${
    public static readonly char[] ending = ".cs".ToCharArray();

	     static string debugInfo = "";
			string PrintDebugInfo(File f){
			debugInfo = string.Join(", ", f.Classes.GroupBy(p => p.Name).Select(p => p.FirstOrDefault()).Select(p => p.Name).ToArray());
          return debugInfo;        
     }

    Template(Settings settings)
    {   
			settings.PartialRenderingMode = PartialRenderingMode.Combined;
        settings.IncludeProject("MyProjectNamespace.Entities");

        settings.OutputFilenameFactory = file =>        {            
			return $"{file.Name.TrimEnd(ending).ToLower()}.gen.ts";       
		};
    }

	 IEnumerable<Class> Imports(File f) {
		var classes = f.Classes;
        return classes.GroupBy(p => p.Name).Select(p => p.FirstOrDefault()).ToArray();
     }
}

module App {$Classes(*)[
    export class $Name { $Properties[
        public $name: $Type;]
    }	
	]
}

//   $PrintDebugInfo

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

8 participants