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

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

Open
roxk opened this issue Oct 30, 2021 · 1 comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request Extensions
Milestone

Comments

@roxk
Copy link
Contributor

roxk commented Oct 30, 2021

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 roxk added bug Something isn't working documentation Improvements or additions to documentation labels Oct 30, 2021
@ghost ghost added the Needs: Triage 🔍 label Oct 30, 2021
@asklar asklar added this to the Backlog milestone Nov 1, 2021
@asklar asklar added enhancement New feature or request and removed bug Something isn't working labels Nov 1, 2021
@asklar
Copy link
Member

asklar commented Nov 1, 2021

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? :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request Extensions
Projects
None yet
Development

No branches or pull requests

2 participants