Forward base class constructors by default

[Tyrrrz]

If you have a custom template base, such as:

internal abstract class MarkdownRazorTemplate<T> : PlainTextTemplate<T>
{
    protected MarkdownRazorTemplate(T model)
        : base(model)
    {
    }

    public new void WriteLiteral(string? literal)
    {
        // ...
    }
}

And a template that inherits from it:

@inherits MarkdownRazorTemplate<MyModel>

Hello @Model.Foo!

And try to instantiate the template like this:

var template = new MyTemplate(new MyModel("world"));

Then the compiler produces an error, saying:

  Program.cs(108, 39): [CS1729] 'MyTemplate' does not contain a constructor that takes 1 arguments

If I modify my template to @inherits RazorBlade.HtmlTemplate<MyModel>, then everything works correctly.

Available workarounds:

• Inline MarkdownRazorTemplate members into MyTempalte. This is not viable for me because I have multiple templates inheriting from it.
• Don't use the model approach and instead use properties. This is not viable for me because I have a lot of shared data between templates, and I'd prefer to pass it around as an object.
• Create partial classes for all templates and add constructors manually. This is what I'm currently doing, although it's not ideal.

[ltrzesniewski]

You need to put a [TemplateConstructor] attribute on the base class' constructor for it to be forwarded in the generated code.

But since you didn't notice this mechanism, maybe I should replace it with an [IgnoreConstructor] attribute instead, and forward all other constructors by default? 🤔

[Tyrrrz]

You need to put a [TemplateConstructor] attribute on the base class' constructor for it to be forwarded in the generated code.

Ah! I even read about it in the readme and forgot 🤦🏻 Sorry.

But since you didn't notice this mechanism, maybe I should replace it with an [IgnoreConstructor] attribute instead, and forward all other constructors by default? 🤔

I'd say forwarding by default when the signature matches seems reasonable.

[ltrzesniewski]

No worries 🙂

I'd say forwarding by default when the signature matches seems reasonable.

Yeah, let's do that.

[Tyrrrz]

You probably don't even have to change the attributes. Essentially, these are the possible scenarios:

• User inherited from RazorTemplateBase<T> but has not provided a constructor to initialize the model with. This should be an error.
• User inherited from RazorTemplateBase<T> but all available constructors don't match the required signature (ctor(T model)). This should be an error.
• User inherited from RazorTemplateBase<T> and provided a matching constructor. Use that constructor as the model initializer.
• User inherited from RazorTemplateBase<T> and provided a matching constructor, but they want to use a different constructor. They should use [TemplateConstructor] to specify a different constructor to use. Although I don't know how realistic this scenario is, because any other constructor they'd provide wouldn't match the signature.

So it looks like the attribute might not be needed at all? Ultimately, if the user inherited from RazorTemplateBase<T> then they have to expose a constructor to set the model from (otherwise what's the point?) 🤔

[ltrzesniewski]

I'm not sure I understand what you're saying. 🤔

I initially added the attribute because of this:

https://github.com/ltrzesniewski/RazorBlade/blob/e5eaecd5ea21a5394aa44732e8c0ad87d8b10848/src/RazorBlade.Library/HtmlTemplate.cs#L189-L195

The designer (of either Rider or VS, I don't remember) didn't like having a base class without a default constructor, and would report errors in the IDE.

So I just wanted to not have this particular constructor in the generated template code. But I didn't want the user to be forced to write a constructor in the Razor file, hence the forwarding mechanism.

• User inherited from RazorTemplateBase<T> but has not provided a constructor to initialize the model with. This should be an error.

That's not what happens today. If the user doesn't provide a constructor, but the base class has a constructor marked with [TemplateConstructor], then the generator will add a constructor with the same signature to the generated class.

As an example of current behavior, if you have the following base class (I'm taking the unit test code as an example):

https://github.com/ltrzesniewski/RazorBlade/blob/e5eaecd5ea21a5394aa44732e8c0ad87d8b10848/src/RazorBlade.Analyzers.Tests/RazorBladeSourceGeneratorTests.cs#L77-L103

The source generator will add this into the generated class:

https://github.com/ltrzesniewski/RazorBlade/blob/e5eaecd5ea21a5394aa44732e8c0ad87d8b10848/src/RazorBlade.Analyzers.Tests/RazorBladeSourceGeneratorTests.should_forward_constructor_from_compilation%23TestNamespace.TestFile.RazorBlade.g.verified.cs#L8-L27

---

And actually... if the user adds a constructor with the same signature as a forwarded constructor from the base class, well, there will be a duplicate constructor. Oops. 😅

I'll have to fix this, especially if the constructors get forwarded by default.

[Tyrrrz]

Ohhh I see. I didn't realize [TemplateConstructor] was used for more than just passing TModel through. In that case, disregard what I said earlier.

Still, if the user derives from e.g. MyTemplateBase<T> which itself derives from HtmlTemplate<T>, then there needs to be a constructor that takes T model and passes it through to RazorTemplateBase<T>.ctor(T model). Because otherwise it's impossible to set Model since it's get-only. Would it make sense to auto-generate a constructor in such case?

[ltrzesniewski]

Would it make sense to auto-generate a constructor in such case?

Nope, if the user wrote a MyTemplateBase<T> class, the source generator won't intervene, as it only generates code for Razor files.

Since MyTemplateBase<T> is written manually, the user (currently) needs to write the following constructor manually:

[TemplateConstructor]
protected MyTemplateBase(T model)
    : base(model)
{
    // ...
}

[Tyrrrz]

What if the user didn't define a constructor on MyTemplateBase<T>, would the template be able to define a constructor that calls into RazorTemplateBase<T>'s constructor?

[ltrzesniewski]

No, since you can't call an ancestor's constructor, you need to call one on your immediate parent.

If you don't declare a constructor on MyTemplateBase<T>, then it'll get a default (parameterless) constructor, which in turn will call into HtmlTemplate<T>'s default constructor, which will throw. Boom. 💥

That's not ideal, but I don't think I can do better without messing up the IDE. 😕

[ltrzesniewski]

Oh, and if RazorBlade forwards constructors by default, you'll still need to write the MyTemplateBase(T model) : base(model) constructor, but without the [TemplateConstructor] attribute.

[Tyrrrz]

I see, that makes sense. Now I understand why you went with this approach.

I think if the user still needs to define an intermediate constructor, then removing the need to specify [TemplateConstructor] won't have such a big impact. Somehow I thought it was possible to call the ancestor's constructor 😅

That said, maybe the source generator can produce a warning if there's an intermediate constructor that isn't marked with [TemplateConstructor] since it's effectively useless at this point.

And as a side note, it would be nice to make WriteLiteral(...) virtual. It's not critical for my use case since I can add new though.

[ltrzesniewski]

That said, maybe the source generator can produce a warning if there's an intermediate constructor that isn't marked with [TemplateConstructor] since it's effectively useless at this point.

Not if constructors get forwarded by default 🙂

And as a side note, it would be nice to make WriteLiteral(...) virtual. It's not critical for my use case since I can add new though.

Right, most other methods are virtual, so that would make sense.

[Tyrrrz]

I made some changes in my project and found a way to consolidate some templates, so constructor forwarding is not required for me anymore 🙂 Let me know if I should close this issue or if you think this is still worth pursuing.

By the way, I'm currently using RazorBlade in two projects:

• DiscordChatExporter: https://github.com/Tyrrrz/DiscordChatExporter/blob/master/DiscordChatExporter.Core/Exporting/MessageGroupTemplate.cshtml
• GitHubActionsTestLogger: https://github.com/Tyrrrz/GitHubActionsTestLogger/blob/master/GitHubActionsTestLogger/TestSummaryTemplate.cshtml

[ltrzesniewski]

After giving it some thought, I think I'll keep the current [TemplateConstructor] mechanism, as I feel it's better as an opt-in feature. That makes RazorBlade only generate the Razor code by default, and library-specific code is only added when explicitly asked for. So yeah, let's close this after all.

I changed a few things thanks to this issue though:

• If you declare a constructor in your template, the generator won't forward a constructor from the base class if its signature would cause a conflict (duplicate constructor). This lets you provide a custom version when needed.
• Only accessible constructors are considered for forwarding (though it's arguable you shouldn't put a [TemplateConstructor] attribute on a private constructor in the first place 😅)
• I made WriteLiteral virtual like you asked for, ~but I'm wondering what I should do with Write(IEncodedContent? content), which is currently not virtual. I think I'll have to make it virtual as well for consistency.~ and I made Write(IEncodedContent? content) virtual as well.

I think I need to document the generator behavior better, and make it very clear that the base classes provided by the library are not special in any way. RazorBlade does not depend on them, and you can write your own whenever you want to. In embedded mode, you can even omit the library entirely with <RazorBladeEmbeddedLibrary>false</RazorBladeEmbeddedLibrary> and go with your own base classes.

The only thing that the generator uses are the attributes, and they serve to emit additional code on top of what Razor generates. I intend to keep the library and the source generator decoupled, except for these attributes.

I'm very happy to know that RazorBlade is useful to you. 🙂

[Tyrrrz]

• I made WriteLiteral virtual like you asked for, ~but I'm wondering what I should do with Write(IEncodedContent? content), which is currently not virtual. I think I'll have to make it virtual as well for consistency.~ and I made Write(IEncodedContent? content) virtual as well.

That's great, thank you! Yeah, it makes sense to make all public/protected methods virtual since it's a class that's meant to be inherited.

In embedded mode, you can even omit the library entirely with <RazorBladeEmbeddedLibrary>false</RazorBladeEmbeddedLibrary> and go with your own base classes.

I rely on this feature heavily in GitHubActionsTestLogger! The templates are baked into the assembly. It would be problematic to have external dependencies for the test logger assembly, since it could cause conflicts with user dependencies.

[ltrzesniewski]

Yes, I noticed you're using this, and it's a very good reason to use it.

But I meant you could even skip the RazorBlade library entirely: it's possible to not integrate it into your project at all. And everything will work fine as long as you provide a properly duck-typed base class of your own. 🙂