Allow @rendermode and @typeparam to coexist in basic scenarios#12591
Allow @rendermode and @typeparam to coexist in basic scenarios#12591davidwengier merged 8 commits intomainfrom
@rendermode and @typeparam to coexist in basic scenarios#12591Conversation
| ["7.0"] = Version_7_0, | ||
| ["8.0"] = Version_8_0, | ||
| ["9.0"] = Version_9_0, | ||
| ["10.0"] = Version_10_0, |
There was a problem hiding this comment.
This feature needs to be tied to .NET 11 in order to not break CI builds, we just have to decide what Razor lang version that is. .NET 10 shipped with RazorLangVersion 9, sadly, and it's not clear to me if its too late to change that so everything lines up nicely:
https://github.com/dotnet/sdk/blob/main/src/RazorSdk/Targets/Sdk.Razor.CurrentVersion.targets#L57
There was a problem hiding this comment.
It seems like we could just introduce a V10 in the .2xx SDK band without any real issues. We might not actually gate anything behind it heh, but at least it would line up. I really like the idea of having a single razor language version per .NET, but that does potentially stop us introducing breaking changes in higher SDK bands 🤔
There was a problem hiding this comment.
Actually, our language versions have a minor part too. So I wonder if we should decide on a policy of major == netversion, and we can introduce minor revisions for things like codgen changes if we need it (Ideally, we solve the code gen thing in a better way, but until then).
There was a problem hiding this comment.
I'm gonna make this v11, since we can't light up this til .net 11 anyway, and we can worry about what to do with v10 later. I don't think there is any reason to not line up major versions.
There was a problem hiding this comment.
hmm, I think "latest" maybe should be v9, but that breaks one of the existing tests. Will bring this up for discussion at the next opportunity
There was a problem hiding this comment.
This feature needs to be tied to .NET 11 in order to not break CI builds, we just have to decide what Razor lang version that is. .NET 10 shipped with RazorLangVersion 9, sadly, and it's not clear to me if its too late to change that so everything lines up nicely:
<RazorLangVersion>preview</RazorLangVersion> 😉
There was a problem hiding this comment.
That'll work if someone has a .NET 11 preview SDK installed (though I intend to make it unnecessary in that case), or if someone wants the IDE to allow it, but their builds to not allow it (and hot reload to not work)
chsienki
left a comment
chsienki
left a comment
There was a problem hiding this comment.
LGTM, modulo a decision on the language versioning.
| ["7.0"] = Version_7_0, | ||
| ["8.0"] = Version_8_0, | ||
| ["9.0"] = Version_9_0, | ||
| ["10.0"] = Version_10_0, |
There was a problem hiding this comment.
It seems like we could just introduce a V10 in the .2xx SDK band without any real issues. We might not actually gate anything behind it heh, but at least it would line up. I really like the idea of having a single razor language version per .NET, but that does potentially stop us introducing breaking changes in higher SDK bands 🤔
| ["7.0"] = Version_7_0, | ||
| ["8.0"] = Version_8_0, | ||
| ["9.0"] = Version_9_0, | ||
| ["10.0"] = Version_10_0, |
There was a problem hiding this comment.
Actually, our language versions have a minor part too. So I wonder if we should decide on a policy of major == netversion, and we can introduce minor revisions for things like codgen changes if we need it (Ideally, we solve the code gen thing in a better way, but until then).
3 participants
Fixes #9683
This doesn't fix the issue in all scenarios, due to limitations of C# (not allowing open generics in attribute names, for example), but the specific cases where this doesn't work, the workaround of defining your own attribute and using
[attribute: ...]also doesn't work, so I think this is a net positive and unblocks the common case for users.