Skip to content

Use WinRT HTTP module in Desktop #6094

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.

Sign up for GitHub

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

Merged
JunielKatarn merged 11 commits into from
Sep 26, 2020
Merged

Use WinRT HTTP module in Desktop #6094

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
7832c19
Use WinRT HTTP module
JunielKatarn Sep 25, 2020
c51ddd1
Remove reference from MSRN.
JunielKatarn Sep 25, 2020
88ce789
Change files
JunielKatarn Sep 25, 2020
6d03f1a
Remove stale source reference
JunielKatarn Sep 25, 2020
6f8827b
Update XHRTest.
JunielKatarn Sep 25, 2020
0f58baa
yarn lint
JunielKatarn Sep 25, 2020
189db97
Re-enable previously failing tests
JunielKatarn Sep 25, 2020
357a76a
Disable IntegrationTestHarness again.
JunielKatarn Sep 25, 2020
0c1bd7a
Treat download as Promise.
JunielKatarn Sep 26, 2020
b45b3f2
yarn lint
JunielKatarn Sep 26, 2020
e32366c
Revert accidentally enabled web debugger
JunielKatarn Sep 26, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions .ado/windows-vs-pr.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -389,13 +389,9 @@ jobs:
variables:
#5059 - Disable failing or intermittent tests (IntegrationTestHarness,AsyncStorage,WebSocket,Logging).
#5265 - WebSocketModuleIntegrationTest::WebSocketModule_Ping fails for Release
#5412 - Disable XHRSample due to intermittedly crashing the test process
Desktop.IntegrationTests.Filter: >
(FullyQualifiedName!=RNTesterIntegrationTests::AsyncStorage)&
(FullyQualifiedName!=RNTesterIntegrationTests::Dummy)&
(FullyQualifiedName!=RNTesterIntegrationTests::IntegrationTestHarness)&
(FullyQualifiedName!=RNTesterIntegrationTests::WebSocket)&
(FullyQualifiedName!=RNTesterIntegrationTests::XHRSample)&
(FullyQualifiedName!~WebSocketModule_)&
(FullyQualifiedName!=WebSocketResourcePerformanceTest::ProcessThreadsPerResource)

Expand Down
8 changes: 8 additions & 0 deletions change/react-native-windows-2020-09-25-01-35-29-consolidate-http.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"type": "prerelease",
"comment": "Use WinRT HTTP module",
"packageName": "react-native-windows",
"email": "julio.rocha@microsoft.com",
"dependentChangeType": "patch",
"date": "2020-09-25T08:35:29.737Z"
}
2 changes: 2 additions & 0 deletions vnext/Desktop.IntegrationTests/RNTesterIntegrationTests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ TEST_CLASS (RNTesterIntegrationTests) {
Assert::AreEqual(L"This is from console.error", result.Message.c_str());
}

BEGIN_TEST_METHOD_ATTRIBUTE(XHRSample)
END_TEST_METHOD_ATTRIBUTE()
TEST_METHOD(XHRSample) {

@NickGerleman NickGerleman Sep 25, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we might have talked about this before, but XHRTest seems to start some async work then immediately pass. Given it was passing before for a mostly stubbed module, I'm not sure how much validation we get from it.

  componentDidMount() {
    try {
      this._download();
    } catch (e) {
      console.error(e);
    }

    TestModule.markTestPassed(true);
  }

@JunielKatarn JunielKatarn Sep 25, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Right now very little:

  • Fail if the native module crashes.
  • Catch any errors during the request send operation.

Now that we can use a complete and proven HTTP module, I'll rewrite this test so it does something relevant (before merging) :)

@NickGerleman NickGerleman Sep 25, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If you're rewriting the test, could you do so in the new IT project instead of the desktop IT's? Should be the same coverage since they're testing the same module.

auto result = m_runner.RunTest("IntegrationTests/XHRTest", "XHRTest");
Assert::AreEqual(TestStatus::Passed, result.Status, result.Message.c_str());
Expand Down
127 changes: 0 additions & 127 deletions vnext/Desktop/Modules/NetworkingModule.cpp
View file
Open in desktop

This file was deleted.

41 changes: 0 additions & 41 deletions vnext/Desktop/Modules/NetworkingModule.h
View file
Open in desktop

This file was deleted.

2 changes: 0 additions & 2 deletions vnext/Desktop/React.Windows.Desktop.vcxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,6 @@
<ClCompile Include="RuntimeOptions.cpp" />
<ClCompile Include="JSBigStringResourceDll.cpp" />
<ClCompile Include="module.g.cpp" />
<ClCompile Include="Modules\NetworkingModule.cpp" />
<ClCompile Include="Modules\TimingModule.cpp" />
<ClCompile Include="Modules\WebSocketModule.cpp" />
<ClCompile Include="pch.cpp">
Expand Down Expand Up @@ -284,7 +283,6 @@
</ClInclude>
<ClInclude Include="..\Microsoft.ReactNative\RedBoxErrorFrameInfo.h" />
<ClInclude Include="..\Microsoft.ReactNative\RedBoxErrorInfo.h" />
<ClInclude Include="Modules\NetworkingModule.h" />
<ClInclude Include="JSBigStringResourceDll.h" />
<ClInclude Include="Modules\TimingModule.h" />
<ClInclude Include="NativeModuleFactories.h" />
Expand Down
6 changes: 0 additions & 6 deletions vnext/Desktop/React.Windows.Desktop.vcxproj.filters
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,6 @@
<ClCompile Include="module.g.cpp">
<Filter>Generated Files</Filter>
</ClCompile>
<ClCompile Include="Modules\NetworkingModule.cpp">
<Filter>Source Files\Modules</Filter>
</ClCompile>
<ClCompile Include="Modules\TimingModule.cpp">
<Filter>Source Files\Modules</Filter>
</ClCompile>
Expand Down Expand Up @@ -195,9 +192,6 @@
<ClInclude Include="JSBigStringResourceDll.h">
<Filter>Header Files</Filter>
</ClInclude>
<ClInclude Include="Modules\NetworkingModule.h">
<Filter>Header Files\Modules</Filter>
</ClInclude>
<ClInclude Include="Modules\TimingModule.h">
<Filter>Header Files\Modules</Filter>
</ClInclude>
Expand Down
1 change: 0 additions & 1 deletion vnext/Microsoft.ReactNative.sln
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,6 @@ Global
Microsoft.ReactNative.Cxx\Microsoft.ReactNative.Cxx.vcxitems*{da8b35b3-da00-4b02-bde6-6a397b3fd46b}*SharedItemsImports = 9
include\Include.vcxitems*{ef074ba1-2d54-4d49-a28e-5e040b47cd2e}*SharedItemsImports = 9
Chakra\Chakra.vcxitems*{f7d32bd0-2749-483e-9a0d-1635ef7e3136}*SharedItemsImports = 4
JSI\Shared\JSI.Shared.vcxitems*{f7d32bd0-2749-483e-9a0d-1635ef7e3136}*SharedItemsImports = 4
Microsoft.ReactNative.Cxx\Microsoft.ReactNative.Cxx.vcxitems*{f7d32bd0-2749-483e-9a0d-1635ef7e3136}*SharedItemsImports = 4
Mso\Mso.vcxitems*{f7d32bd0-2749-483e-9a0d-1635ef7e3136}*SharedItemsImports = 4
Shared\Shared.vcxitems*{f7d32bd0-2749-483e-9a0d-1635ef7e3136}*SharedItemsImports = 4
Expand Down
5 changes: 4 additions & 1 deletion vnext/Microsoft.ReactNative/Base/CoreNativeModules.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,10 @@ std::vector<facebook::react::NativeModuleDescription> GetCoreModules(
[]() { return std::make_unique<react::uwp::WebSocketModule>(); },
jsMessageQueue);

modules.emplace_back(NetworkingModule::Name, []() { return std::make_unique<NetworkingModule>(); }, jsMessageQueue);
modules.emplace_back(
Microsoft::React::NetworkingModule::Name,
[]() { return std::make_unique<Microsoft::React::NetworkingModule>(); },
jsMessageQueue);

modules.emplace_back(
"Timing",
Expand Down
2 changes: 0 additions & 2 deletions vnext/Microsoft.ReactNative/Microsoft.ReactNative.vcxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,6 @@
<ClInclude Include="Modules\LinkingManagerModule.h" />
<ClInclude Include="Modules\LogBoxModule.h" />
<ClInclude Include="Modules\NativeUIManager.h" />
<ClInclude Include="Modules\NetworkingModule.h" />
<ClInclude Include="Modules\TimingModule.h" />
<ClInclude Include="Modules\WebSocketModuleUwp.h" />
<ClInclude Include="NativeModulesProvider.h" />
Expand Down Expand Up @@ -440,7 +439,6 @@
<ClCompile Include="Modules\LinkingManagerModule.cpp" />
<ClCompile Include="Modules\LogBoxModule.cpp" />
<ClCompile Include="Modules\NativeUIManager.cpp" />
<ClCompile Include="Modules\NetworkingModule.cpp" />
<ClCompile Include="Modules\TimingModule.cpp" />
<ClCompile Include="Modules\WebSocketModuleUwp.cpp" />
<ClCompile Include="NativeModulesProvider.cpp" />
Expand Down
6 changes: 0 additions & 6 deletions vnext/Microsoft.ReactNative/Microsoft.ReactNative.vcxproj.filters
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,9 +167,6 @@
<ClCompile Include="Modules\NativeUIManager.cpp">
<Filter>Modules</Filter>
</ClCompile>
<ClCompile Include="Modules\NetworkingModule.cpp">
<Filter>Modules</Filter>
</ClCompile>
<ClCompile Include="Modules\TimingModule.cpp">
<Filter>Modules</Filter>
</ClCompile>
Expand Down Expand Up @@ -518,9 +515,6 @@
<ClInclude Include="Modules\NativeUIManager.h">
<Filter>Modules</Filter>
</ClInclude>
<ClInclude Include="Modules\NetworkingModule.h">
<Filter>Modules</Filter>
</ClInclude>
<ClInclude Include="Modules\TimingModule.h">
<Filter>Modules</Filter>
</ClInclude>
Expand Down
1 change: 0 additions & 1 deletion vnext/ReactWindows-Desktop.sln
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ Global
Shared\Shared.vcxitems*{2049dbe9-8d13-42c9-ae4b-413ae38fffd0}*SharedItemsImports = 9
Microsoft.ReactNative.Cxx\Microsoft.ReactNative.Cxx.vcxitems*{44dced9b-9c4c-48fe-8545-0930192bbc16}*SharedItemsImports = 4
Mso\Mso.vcxitems*{44dced9b-9c4c-48fe-8545-0930192bbc16}*SharedItemsImports = 4
Chakra\Chakra.vcxitems*{6f354505-fe3a-4bd2-a9a6-d12bbf37a85c}*SharedItemsImports = 4
Mso\Mso.vcxitems*{84e05bfa-cbaf-4f0d-bfb6-4ce85742a57e}*SharedItemsImports = 9
Chakra\Chakra.vcxitems*{95048601-c3dc-475f-adf8-7c0c764c10d5}*SharedItemsImports = 4
Microsoft.ReactNative.Cxx\Microsoft.ReactNative.Cxx.vcxitems*{95048601-c3dc-475f-adf8-7c0c764c10d5}*SharedItemsImports = 4
Expand Down
5 changes: 3 additions & 2 deletions ....ReactNative/Modules/NetworkingModule.cpp → vnext/Shared/Modules/NetworkingModule.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include "pch.h"

#include <Utils/CppWinrtLessExceptions.h>
#include <winrt/Windows.Foundation.Collections.h>
#include <winrt/Windows.Security.Cryptography.h>
#include <winrt/Windows.Storage.Streams.h>
#include <winrt/Windows.Web.Http.Headers.h>
Expand All @@ -25,7 +26,7 @@
#pragma optimize("", off)
#endif

namespace react::uwp {
namespace Microsoft::React {

//
// NetworkingModule::NetworkingHelper
Expand Down Expand Up @@ -423,4 +424,4 @@ auto NetworkingModule::getMethods() -> std::vector<Method> {
};
}

} // namespace react::uwp
} // namespace Microsoft::React
4 changes: 2 additions & 2 deletions ...ft.ReactNative/Modules/NetworkingModule.h → vnext/Shared/Modules/NetworkingModule.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
#include <memory>
#include <vector>

namespace react::uwp {
namespace Microsoft::React {

class NetworkingModule : public facebook::xplat::module::CxxModule {
public:
Expand All @@ -31,4 +31,4 @@ class NetworkingModule : public facebook::xplat::module::CxxModule {
std::shared_ptr<NetworkingHelper> m_networking;
};

} // namespace react::uwp
} // namespace Microsoft::React
2 changes: 2 additions & 0 deletions vnext/Shared/Shared.vcxitems
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
</ClCompile>
<ClCompile Include="$(MSBuildThisFileDirectory)Modules\ExceptionsManagerModule.cpp" />
<ClCompile Include="$(MSBuildThisFileDirectory)Modules\I18nModule.cpp" />
<ClCompile Include="$(MSBuildThisFileDirectory)Modules\NetworkingModule.cpp" />
<ClCompile Include="$(MSBuildThisFileDirectory)Modules\PlatformConstantsModule.cpp" />
<ClCompile Include="$(MSBuildThisFileDirectory)Modules\SourceCodeModule.cpp" />
<ClCompile Include="$(MSBuildThisFileDirectory)Modules\StatusBarManagerModule.cpp" />
Expand All @@ -64,6 +65,7 @@
<ClInclude Include="$(MSBuildThisFileDirectory)AsyncStorage\AsyncStorageManager.h" />
<ClInclude Include="$(MSBuildThisFileDirectory)AsyncStorage\FollyDynamicConverter.h" />
<ClInclude Include="$(MSBuildThisFileDirectory)AsyncStorage\KeyValueStorage.h" />
<ClInclude Include="$(MSBuildThisFileDirectory)Modules\NetworkingModule.h" />
<ClInclude Include="$(MSBuildThisFileDirectory)RuntimeOptions.h" />
<ClInclude Include="$(MSBuildThisFileDirectory)Modules\AsyncStorageModuleWin32.h" />
<ClInclude Include="$(MSBuildThisFileDirectory)AsyncStorage\StorageFileIO.h" />
Expand Down
8 changes: 7 additions & 1 deletion vnext/Shared/Shared.vcxitems.filters
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,9 @@
<Filter>Source Files\Executors</Filter>
</ClCompile>
<ClCompile Include="$(MSBuildThisFileDirectory)PackagerConnection.cpp" />
<ClCompile Include="$(MSBuildThisFileDirectory)Modules\NetworkingModule.cpp">
<Filter>Source Files\Modules</Filter>
</ClCompile>
</ItemGroup>
<ItemGroup>
<Filter Include="Source Files">
Expand Down Expand Up @@ -306,6 +309,9 @@
<ClInclude Include="$(MSBuildThisFileDirectory)RuntimeOptions.h">
<Filter>Header Files</Filter>
</ClInclude>
<ClInclude Include="$(MSBuildThisFileDirectory)Modules\NetworkingModule.h">
<Filter>Header Files\Modules</Filter>
</ClInclude>
</ItemGroup>
<ItemGroup>
<None Include="$(MSBuildThisFileDirectory)etw\build.bat">
Expand All @@ -321,4 +327,4 @@
<Filter>Source Files\etw</Filter>
</None>
</ItemGroup>
</Project>
</Project>
Loading