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

Toolkit Doesn't Detect UnSupported C# Version or Doesn't work on C# 8 #376

Closed
1 task done
AathifMahir opened this issue Aug 10, 2022 · 7 comments · Fixed by #398
Closed
1 task done

Toolkit Doesn't Detect UnSupported C# Version or Doesn't work on C# 8 #376

AathifMahir opened this issue Aug 10, 2022 · 7 comments · Fixed by #398
Labels
bug 🐛 An unexpected issue that highlights incorrect behavior mvvm-toolkit 🧰 Issues/PRs for the MVVM Toolkit

Comments

@AathifMahir
Copy link

AathifMahir commented Aug 10, 2022

Describe the bug

Community Toolkit MVVM Doesn't Support C# Version 8 and Does Only Work with C# Version 9 and Above at the time of writing, In this case there's no proper warning or error output that relates to Community Toolkit MVVM on Compilation

Regression

7.1.2

Reproducible in sample app?

  • This bug can be reproduced in the sample app.

Steps to reproduce

1. Create Vanilla UWP Project
2. Install Community Toolkit 8.0 using Nuget
3. Compile the App

Expected behavior

Community Toolkit MVVM Should Have Language Barrier to Prevent the Dependency from Installing on C# version 8 since least version of C# that is supported by Community Toolkit MVVM is C# Version 9 at the time of writing or Community Toolkit MVVM Should Support C# version 8

Screenshots

Screenshot 2022-08-10 1303041

Visual Studio Version

2022

Visual Studio Build Number

17.3

Nuget packages

Community Toolkit 8.0

Help us help you

Yes, but only if others can assist.

@AathifMahir AathifMahir added the bug 🐛 An unexpected issue that highlights incorrect behavior label Aug 10, 2022
@ghost ghost added the needs triage 🔍 This issue is awaiting triage by maintainers label Aug 10, 2022
@ghost
Copy link

ghost commented Aug 10, 2022

Hello AathifMahir, thank you for opening an issue with us!

I have automatically added a "needs triage" label to help get things started. Our team will analyze and investigate the issue, and escalate it to the relevant team if possible. Other community members may also look into the issue and provide feedback 🙌

@AathifMahir AathifMahir changed the title Toolkit Doesn't Detect UnSupported C# Version Toolkit Doesn't Detect UnSupported C# Version or Doesn't work on C# 8 Aug 10, 2022
@Arlodotexe Arlodotexe added visual studio 🔀 and removed needs triage 🔍 This issue is awaiting triage by maintainers labels Aug 10, 2022
@Arlodotexe
Copy link
Member

@AathifMahir Are you saying Visual Studio should prevent you from installing the Toolkit if you're using an unsupported C# version?

@AathifMahir
Copy link
Author

AathifMahir commented Aug 10, 2022

@AathifMahir Are you saying Visual Studio should prevent you from installing the Toolkit if you're using an unsupported C# version?

Nope, I was saying the opposite of that, I mean community toolkit should present a language barrier or output the issue after Initialization or installation if possible, seems like I wrote the issue in a complex way. Let me edit that

Edit: Fixed and Updated the Issue

@michael-hawker michael-hawker transferred this issue from CommunityToolkit/WindowsCommunityToolkit Aug 10, 2022
@michael-hawker michael-hawker added the mvvm-toolkit 🧰 Issues/PRs for the MVVM Toolkit label Aug 10, 2022
@michael-hawker
Copy link
Member

@AathifMahir there's many community toolkits, so you need to be more specific. But I'm pretty sure you're talking about the MVVM package. I think you and Sergio may have been talking about this on Discord?

There's supposed to be a specific analytic message which instructs the developer about this change, but appears to not be showing up here.

@Sergio0694
Copy link
Member

I think I forgot to update the generated code in the cached args when I lowered the minimum language version to C# 8. I reckon removing the target-typed new expressions from there should be enough, and then we can keep C# 8 as minimum. The check there is already in place, it's just that it's not enough because that generated code accidentally still used C# 9 features.

Should be an easy fix, will take a look in a bit 🙂
I reckon it should be enough to just replace this with an initializer taking the identifier name explicitly:

VariableDeclarator(Identifier(propertyName))
.WithInitializer(EqualsValueClause(
ImplicitObjectCreationExpression()
.AddArgumentListArguments(Argument(
LiteralExpression(SyntaxKind.StringLiteralExpression, Literal(propertyName))))))))

@AathifMahir
Copy link
Author

@AathifMahir there's many community toolkits, so you need to be more specific. But I'm pretty sure you're talking about the MVVM package. I think you and Sergio may have been talking about this on Discord?

There's supposed to be a specific analytic message which instructs the developer about this change, but appears to not be showing up here.

Took care of it 👍

@zsolt3991
Copy link

Is vanilla UWP still under support with the new 8.x.x versions of the Mvvm Toolkit? Even C#8 as minimum version would require manually bumping the LanguageVersion of the consuming project, which brings some risks that have to be managed relating to some of the C#8 features, since UWP is normally stuck on 7.3.
Or should UWP consumers refrain from using the Source Generator based functionalities, since the code generated adheres to the C#8 spec and even this could be subject to change when .Net Core 3.1 is deprecated in November?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 An unexpected issue that highlights incorrect behavior mvvm-toolkit 🧰 Issues/PRs for the MVVM Toolkit
Projects
None yet
5 participants