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

Update libraries for .NET Core to 2.0 and for .NET Standard to 2.0 #436

Merged
merged 2 commits into from
Nov 4, 2017

Conversation

ShikiGami
Copy link
Contributor

  • Support for ASP.NET Core targeting .NET Framework to 4.6.1
  • The use of .NET Standard 2.0 means .NET Framework is no longer required for React.AspNet and React.AspNet.Middleware

- Support for ASP.NET Core targeting .NET Framework to 4.6.1
- The use of .NET Standard 2.0 means .NET Framework is no longer required for React.AspNet and React.AspNet.Middleware
- Since Microsoft.AspNetCore.All doesn't appear to mark .NET Framework 4.6.1 dependencies, I went and changed it back to individual package referencing
@@ -5,7 +5,7 @@
<Copyright>Copyright 2014-Present Facebook, Inc</Copyright>
<AssemblyTitle>ReactJS.NET - Babel middleware for ASP.NET Core</AssemblyTitle>
<Authors>Daniel Lo Nigro</Authors>
<TargetFrameworks>net451;netstandard1.6</TargetFrameworks>
<TargetFrameworks>netstandard2.0</TargetFrameworks>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add target 451 or 461 too

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wrote it on the pull request description, but with netstandard2.0 you do not requiere a different target if you use the library in a application targeting .NET Framework 4.6.1, that's the whole point of .NET Standard.

@Daniel15
Copy link
Member

Daniel15 commented Oct 9, 2017

I'm so sorry for the delay... Not sure how I missed this PR! I didn't think someone had submitted one yet!

It looks like the AppVeyor build failed... Is it related? https://ci.appveyor.com/project/Daniel15/react-net/build/271

@@ -1,4 +1,4 @@
/*
/*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What changed on this line? Is it a Byte Order Mark?

<PropertyGroup Condition=" '$(Configuration)' == 'Release' ">
<TreatWarningsAsErrors>true</TreatWarningsAsErrors>
</PropertyGroup>

<PropertyGroup>
<NoWarn>7035</NoWarn>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. The build script passes this in, but I guess the warning still appears when building in Visual Studio.

@Daniel15
Copy link
Member

Daniel15 commented Oct 9, 2017

This looks pretty good to me! Let's get the AppVeyor build green, and then I'll merge it. You might need to modify the AppVeyor config .yml file (eg. if it needs to be changed to a .NET Core 2.0 AppVeyor image)

@dustinsoftware
Copy link
Member

Now that React.Router.csproj exists, it would be good to update that as well.

@Daniel15
Copy link
Member

Daniel15 commented Nov 4, 2017

I manually merged this as I had to make some changes to the "classic" ASP.NET samples (React.Sample.Mvc4, React.Sample.Webpack and React.Sample.Cassette) to get them to compile: dd4e504

@dustinsoftware - I'll upgrade React.Router.csproj separately. I suspect it'll need two separate csproj files - One for ASP.NET 4.x and one for ASP.NET Core. Currently it seems to assume that .NET Framework 4.6.1 = ASP.NET 4.x and netstandard = ASP.NET Core, but you can use ASP.NET Core on .NET Framework 4.6.1.

@Daniel15
Copy link
Member

Daniel15 commented Nov 4, 2017

@dustinsoftware See #461

@Daniel15
Copy link
Member

Daniel15 commented Nov 8, 2017

Released this today as version 3.2. Thanks!

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

Successfully merging this pull request may close these issues.

4 participants