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

DO NOT MERGE - refactor(ReactNative): Refactoring project to use .NET Standard #1419

Closed
wants to merge 21 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@rozele
Member

rozele commented Oct 6, 2017

This is a start to the migration to a .NET Standard library for the core React Native bridge. The hope is that many other platforms can extend the .NET Standard bridge with their own threading models, JavaScript runtimes, and UIManagers (think JavaScriptCore, Xamarin.Mac, Gtk# options).

I want to make this process very incremental, move one component / interface at a time. This will give us an opportunity to eliminate dead code, ensure C# styles and best practices are being used, and clean up the overall developer experience.

cc @matthargett, I'd like to start making PRs against the netstandard branch and we can rebase them all onto master once we have a reasonable chunk of the library ready.

rozele and others added some commits Oct 6, 2017

refactor(ReactNative): Refactoring project to use .NET Standard
This is a start to the migration to a .NET Standard library for the core React Native bridge. The hope is that many other platforms can extend the .NET Standard bridge with their own threading models, JavaScript runtimes, and UIManagers (think JavaScriptCore, Xamarin.Mac, Gtk# options).

I want to make this process very incremental, move one component / interface at a time. This will give us an opportunity to eliminate dead code, ensure C# styles and best practices are being used, and clean up the overall developer experience.
Add initial unit test for new ReactNative.Core project (#1449)
* Add initial unit test for new ReactNative.Core project
* Update to VS2017 where unit testing of .NET Core actually works
* Bump NUnit whose nuget packages work around new tooling bugs
* Bump Rx.NET to work around weird mapping that no longer works
* Switch AppVeyor to VS2017 to match master to support .NET Core 2.0, required for the added unit tests.

* Upgrade projects to VS2017. Consistently update minimum UWP version to Anniversary Edition and target Fall Creators update, and UniversalWindowsPlatform NuGet to 6.0.1. Consistently use latest NUnit and NUnit3Adapter. Lower netstandard target for ReactNative.Core to 1.2 (.NET 4.5.1 compatible). ReactNative.Core and its Tests both compile and run in VS2017 Mac and Windows. I had problems getting some UWP Unit Tests to run due to a deploy issue, but I think it's unrelated.

* NUnit [DefaultFloatingPointTolerance()] attribute only applies to [Test], not to entire fixture. Fixes one of the CI failures.

* Upgrade to latest Rx.NET to fix bizarre build issues. Now the UWP Tests won't deploy locally for me, but let's see how they do on CI. All tests except UWP run in VS2017 Windows.

* Adjust appveyor.yml to point at newer nunit-console location. Bump NUnit version.

* Adjust TargetPlatformVersion based on feedback from @rozele.
Merge from master. Move minimal necessary types into Core. Same tests…
… can't be moved into Core.Tests as-is because the tests use Windows-specific namespaces.
Move all easy Bridge and Reflection files to netstandard Core project…
…. Took notes on why other ones are difficult, would like to discuss tactics for moving some over. All tests build and run in VS2017 Windows, Core tests build and run in VS2017 Mac.
@codecov-io

This comment has been minimized.

codecov-io commented Nov 30, 2017

Codecov Report

Merging #1419 into master will decrease coverage by 1.16%.
The diff coverage is 77.77%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1419      +/-   ##
==========================================
- Coverage   31.38%   30.22%   -1.17%     
==========================================
  Files         268      227      -41     
  Lines       19631    17623    -2008     
  Branches     1640     1503     -137     
==========================================
- Hits         6162     5327     -835     
+ Misses      13311    12177    -1134     
+ Partials      158      119      -39
Impacted Files Coverage Δ
....Tests/Bridge/ReactContextNativeModuleBaseTests.cs 80% <ø> (ø) ⬆️
...ReactNative.Net46/Modules/NetInfo/NetInfoModule.cs 77.5% <ø> (ø) ⬆️
...ctNative.Shared/Modules/AppState/AppStateModule.cs 84.84% <ø> (ø) ⬆️
...tNative.Net46/Modules/WebSocket/WebSocketModule.cs 66.03% <ø> (ø) ⬆️
...ative.Shared/UIManager/UIImplementationProvider.cs 100% <ø> (ø) ⬆️
...Windows/ReactNative.Shared/ReactInstanceManager.cs 0% <ø> (ø) ⬆️
...ve.Shared/Modules/Core/DeviceEventManagerModule.cs 0% <ø> (ø) ⬆️
...s/ReactNative.Net46/Modules/Dialog/DialogModule.cs 0% <ø> (ø) ⬆️
...indows/ReactNative.Net46/Shell/MainReactPackage.cs 0% <ø> (ø) ⬆️
...ctWindows/ReactNative.Shared/CoreModulesPackage.cs 0% <ø> (ø) ⬆️
... and 22 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4483d33...064f22b. Read the comment docs.

matthargett and others added some commits Dec 7, 2017

Introduce IReactContext to encapsulate concrete ReactContext type whi…
…ch currently references concrete DispatcherHelpers that is filled in by platform-specific projects. Move additional files. Next step is to try moving files previously held back by their concrete reference to ReactContext (NativeModuleRegistry, ReactInstance, etc). Possibly try converting #if in JavaScriptBundleLoader to subclasses.
print out node version. only run ReactNative.Core.Tests, as disocveri…
…ng 0 tests in the Core library itself causes dotnet test to exit non-zero.
use store_test_results config entry instead of copying xml file. add …
…comment about why we have to use an aspnetcore container.
Move Promise tests and MockCallback into core (#1550)
* Move Promise tests and MockCallback into core
* Update namespaces and assert syntax
* Temporarily disable lint, as in master, as it is crashing
@rozele

This comment has been minimized.

Member

rozele commented Mar 27, 2018

Leaving branch, closing PR.

@rozele rozele closed this Mar 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment