codegen - 0.74: Int32 | null generates Nullable Integer on java side, but Ljava/lang/Double on c++ side. about react-native HOT 12 CLOSED

I've tested it with patch-package, and it worked fine.

Great thanks for the confirmation 👍

Note that this fix, while looks great and is correct. Can actually cause issues in some case

We're aware of this. That's why this change was marked as breaking + started from last version we added support for includesGeneratedCode:

That will allow libraries to ship the generated code so that mismatch between codegen versions like this one won't happen again

from react-native.

We ended up reverting the partial Int32/Float implementation, as attempting to fix it with the previous PR I linked would create more problems with libraries like -svg and others.

We'll get back to it in a future release.

from react-native.

from react-native.

Is this a regression from 0.73 to 0.74 @mfazekas ?

from react-native.

@cortinico yes I believe so. At least the error - Error: Exception in HostFunction: no non-static method "Lcom/rnmapbox/rnmbx/components/mapview/NativeMapViewModule;.setHandledMapChangedEvents(Ljava/lang/Double;Lcom/facebook/react/bridge/ReadableArray;Lcom/facebook/react/bridge/Promise;)V we've not encountered with 0.73.

But looking at the sources this part was not touched in 4 years.
https://github.com/facebook/react-native/blame/9922628032ace4d87257c1b3d70c2dc6c38a60a6/packages/react-native-codegen/src/generators/modules/GenerateModuleJniCpp.js#L262-L263

Also in 0.73.0 I also see double

static facebook::jsi::Value __hostFunction_NativeMapViewModuleSpecJSI_setHandledMapChangedEvents(facebook::jsi::Runtime& rt, TurboModule &turboModule, const facebook::jsi::Value* args, size_t count) {
  static jmethodID cachedMethodId = nullptr;
  return static_cast<JavaTurboModule &>(turboModule).invokeJavaMethod(rt, PromiseKind, "setHandledMapChangedEvents", "(Ljava/lang/Double;Lcom/facebook/react/bridge/ReadableArray;Lcom/facebook/react/bridge/Promise;)V", args, count, cachedMethodId);
}

So not sure why we started to face the issue now.

from react-native.

@mfazekas Could we get a small repro using https://github.com/react-native-community/reproducer-react-native ? That would make it way easier for me to understand what happened.

from react-native.

@cortinico sure will try to do that

from react-native.

@cortinico so the issue is that in RN 0.73
GenerateModuleJavaSpec.js has generated

public abstract void setHandledMapChangedEvents(@Nullable Double viewRef, ReadableArray events, Promise promise); 

while RN 0.74 it's changed to:

- public abstract void setHandledMapChangedEvents(@Nullable Double viewRef, ReadableArray events, Promise promise); 
+ public abstract void setHandledMapChangedEvents(@Nullable Integer viewRef, ReadableArray events, Promise promise);

see
ccd3b04
#42126

But rest of RN 0.74 has not made this change.

from react-native.

As for if it's a breaking change? I'm not 100% sure many modules like react-native-svg are generating it once, and haven't rerun with the changed codegen in RN 0.74. We've reran with @rnmapbox/maps that's why we've faced the issue.

https://github.com/software-mansion/react-native-svg/blob/main/android/src/paper/java/com/horcrux/svg/NativeSvgRenderableModuleSpec.java

from react-native.

This is indeed a bug. Fix for it is here:

• #45024

@mfazekas if you could try it with patch-package and report back, would be gold. We could include it in the next 0.74 patch release.

from react-native.

@cortinico
I've tested it with patch-package, and it worked fine.
mfazekas/maps-1@dbcf2b1

Note that this fix, while looks great and is correct. Can actually cause issues in some case - like, the mentioned react-native-svg has the spec generated with 0.73 or even older, so it might work because of the current but, but with next 0.74.* release they'll need to update those generated java code: https://github.com/software-mansion/react-native-svg/blob/main/android/src/paper/java/com/horcrux/svg/NativeSvgRenderableModuleSpec.java

from react-native.

Ok so for React Native SVG the fix would be here:

• software-mansion/react-native-svg#2309

@mfazekas have you noticed any similar failures?

from react-native.

@cortinico
So, it will not be possible to use Int when mapping from TS to Java/Kotlin? (Sincere doubt)

from react-native.