react-native-windows-samples icon indicating copy to clipboard operation
react-native-windows-samples copied to clipboard

Native module's first example showcase incorrect usage of `REACT_METHOD`. It's paired with a method without a promise as one its argument

Open roxk opened this issue 2 years ago • 1 comments

Page url

https://microsoft.github.io/react-native-windows/docs/native-modules

Problem Description

REACT_METHOD requires a promise as one of the arguments, but the doc sample at "1. Authoring your Native Module" has the following methods

REACT_METHOD(Add, L"add");
double Add(double a, double b) noexcept
{
  double result = a + b;
  AddEvent(result);
  return result;
}

which, when it is paired with the following ts, isn't working:

const math = NativeModules.FancyMath
const value = math.add(1, 0)
console.log(`value=${value}`);
// Output value=undefined

Changing the above code as follows fixes the issue:

// c++
REACT_METHOD(Add, L"add");
void Add(double a, double b, winrt::Microsoft::ReactNative::ReactPromise<double>&& result) noexcept
{
  result.Resolve(a + b);
}

// typescript
const math = NativeModules.FancyMath
const value = await math.add(1, 0)
console.log(`value=${value}`);
// Output value = 1

How I Discovered The Issue

Reading the doc, I thought 0-arity method would work like so:

// .cpp
REACT_METHOD(Test, "test")
JSValue Test() noexcept
{
  return {};
}

// .tsx
const module = NativeModules.MyModule
const value = module.test()

However, upon debugging, an exception is thrown at CxxNativeModule.cpp instead: image The intended exception message was "Expected 1 callbacks, but only 0 parameters provided".

Note that I did read REACT_METHOD was async and REACT_SYNC_METHOD was synchronous. But that double Add(double, double) example above mislead me into writing the above code.

Suggested fix

Replace

REACT_METHOD(Add, L"add");
double Add(double a, double b) noexcept
{
  double result = a + b;
  AddEvent(result);
  return result;
}

with

REACT_METHOD(Add, L"add");
void Add(double a, double b, winrt::Microsoft::ReactNative::ReactPromise<double>&& promise) noexcept
{
  double result = a + b;
  AddEvent(result);
  promise.Resolve(result);
}

Info

System:
    OS: Windows 10 10.0.22000
    CPU: (16) x64 AMD Ryzen 7 2700X Eight-Core Processor
    Memory: 6.66 GB / 15.92 GB
  Binaries:
    Node: 14.18.0 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.19.1 - C:\Program Files (x86)\Yarn\bin\yarn.CMD
    npm: 6.14.15 - C:\Program Files\nodejs\npm.CMD
    Watchman: Not Found
  SDKs:
    Android SDK:
      API Levels: 27, 28, 29, 30
      Build Tools: 28.0.3, 29.0.2, 29.0.3
      Android NDK: Not Found
    Windows SDK:
      AllowDevelopmentWithoutDevLicense: Enabled
      AllowAllTrustedApps: Enabled
      Versions: 10.0.16299.0, 10.0.17763.0, 10.0.18362.0, 10.0.19041.0, 10.0.22000.0
  IDEs:
    Android Studio: Version  4.1.0.0 AI-201.8743.12.41.6858069
    Visual Studio: 16.11.31729.503 (Visual Studio Community 2019), 17.0.31410.414 (Visual Studio Community 2022)
  Languages:
    Java: 1.8.0_262 - C:\Program Files\AdoptOpenJDK\jdk-8.0.262.10-hotspot\bin\javac.EXE
    Python: 3.7.3 - C:\Users\Name\AppData\Local\Programs\Python\Python37\python.EXE
  npmPackages:
    @react-native-community/cli: Not Found
    react: 16.13.1 => 16.13.1 
    react-native: 0.63.4 => 0.63.4 
    react-native-windows: 0.63.41 => 0.63.41 
  npmGlobalPackages:
    *react-native*: Not Found

roxk avatar Oct 30 '21 03:10 roxk

The REACT_METHOD macro works with a return value (no promise) when the JS side is using the method via a callback, like: https://github.com/microsoft/react-native-windows/blob/main/packages/sample-apps/index.windows.js#L214

However, it won't work if you are trying to use it in an await, but the docs don't clarify this distinction.

So the docs are "right" as long as you use callbacks, not await : ) It's worth then clarifying this in the docs though. Mind adding that blurb? :)

asklar avatar Nov 01 '21 18:11 asklar