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

[iOS] Codegen: Generates broken code for event emitter with nested objects in arrays #47113

Closed
hannojg opened this issue Oct 18, 2024 · 4 comments
Labels
Issue: Author Provided Repro This issue can be reproduced in Snack or an attached project. Platform: iOS iOS applications. Resolution: Fixed A PR that fixes this issue has been merged. Tech: Codegen Related to react-native-codegen Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules)

Comments

@hannojg
Copy link
Contributor

hannojg commented Oct 18, 2024

Description

When having a fabric view component spec and using a direct event emitter which returns an object, which has an array field, which is of a custom object type the generates C++ code for the EventEmitter is broken.

This is the spec for example:

export interface NativeProps extends ViewProps {
  onCodeScanned?: DirectEventHandler<
    Readonly<{
      codes: Readonly<
        {
          type: string
          frame?: Readonly<{ x: Double; y: Double; width: Double; height: Double }>
          corners?: Readonly<{ x: Double; y: Double }[]>
        }[]
      >
      frame: Readonly<{ width: Int32; height: Int32 }[]>
    }>
  >
}

export default codegenNativeComponent<NativeProps>('CameraView')

And this is code generated by Codegen:

Screenshot 2024-10-18 at 08 55 04

To me it looks like it inserted a , where it should have used a ..

Steps to reproduce

  1. Clone the reproducer repo: https://github.com/hannojg/reproducer-react-native
  2. cd ReproducerApp
  3. yarn
  4. npx react-native codegen
  5. open ios/build/generates/ios/react/RNCameraSpec/EventEmitters.cpp, or try to build iOS

React Native Version

0.75.4

Affected Platforms

Build - MacOS, Build - Windows, Build - Linux

Output of npx react-native info

System:
  OS: macOS 15.0.1
  CPU: (12) arm64 Apple M2 Pro
  Memory: 85.53 MB / 32.00 GB
  Shell:
    version: "5.9"
    path: /bin/zsh
Binaries:
  Node:
    version: 20.17.0
    path: ~/.nvm/versions/node/v20.17.0/bin/node
  Yarn:
    version: 1.22.22
    path: ~/.nvm/versions/node/v20.17.0/bin/yarn
  npm:
    version: 10.8.2
    path: ~/.nvm/versions/node/v20.17.0/bin/npm
  Watchman: Not Found
Managers:
  CocoaPods:
    version: 1.15.2
    path: /Users/hannomargelo/.rbenv/shims/pod
SDKs:
  iOS SDK:
    Platforms:
      - DriverKit 24.0
      - iOS 18.0
      - macOS 15.0
      - tvOS 18.0
      - visionOS 2.0
      - watchOS 11.0
  Android SDK:
    API Levels:
      - "28"
      - "30"
      - "31"
      - "32"
      - "33"
      - "33"
      - "34"
    Build Tools:
      - 28.0.3
      - 30.0.2
      - 30.0.3
      - 31.0.0
      - 33.0.0
      - 33.0.1
      - 33.0.2
      - 34.0.0
      - 35.0.0
    System Images:
      - android-32 | Google APIs ARM 64 v8a
      - android-33 | Wear OS 4 ARM 64 v8a
      - android-33 | Google APIs ARM 64 v8a
      - android-33 | Google Play ARM 64 v8a
      - android-33 | Google APIs ATD ARM 64 v8a
      - android-34 | Google APIs ARM 64 v8a
      - android-34 | Google APIs ATD ARM 64
    Android NDK: 26.1.10909125
IDEs:
  Android Studio: 2024.2 AI-242.23339.11.2421.12483815
  Xcode:
    version: 16.0/16A242d
    path: /usr/bin/xcodebuild
Languages:
  Java:
    version: 17.0.7
    path: /Users/hannomargelo/.jenv/shims/javac
  Ruby:
    version: 2.7.6
    path: /Users/hannomargelo/.rbenv/shims/ruby
npmPackages:
  "@react-native-community/cli": Not Found
  react: Not Found
  react-native: Not Found
  react-native-macos: Not Found
npmGlobalPackages:
  "*react-native*": Not Found
Android:
  hermesEnabled: true
  newArchEnabled: true
iOS:
  hermesEnabled: true
  newArchEnabled: true

Stacktrace or Logs

There are no relevant logs, the codegen command executes fine.

Reproducer

https://github.com/hannojg/reproducer-react-native

Screenshots and Videos

No response

@hannojg
Copy link
Contributor Author

hannojg commented Oct 18, 2024

Update: I was able to circumvent this by removing the ReadOnly

Update: not sure what it was even w/o readonly its broken 😅

@cortinico cortinico added Tech: Codegen Related to react-native-codegen Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules) labels Oct 18, 2024
@cortinico cortinico changed the title Codegen: Generates broken code for event emitter with nested objects in arrays [iOS] Codegen: Generates broken code for event emitter with nested objects in arrays Oct 18, 2024
@cortinico cortinico added Platform: iOS iOS applications. Issue: Author Provided Repro This issue can be reproduced in Snack or an attached project. and removed Needs: Triage 🔍 labels Oct 18, 2024
@cortinico
Copy link
Contributor

Just tested on Android and this does not reproduce (most likely because we don't use the event emitter generated by codegen in Android)

hannojg added a commit to mrousavy/react-native-vision-camera that referenced this issue Oct 28, 2024
@tvanlaerhoven
Copy link
Contributor

I was able to reproduce this as well. It seems the property parts are not spread correctly before joining. I opened a PR.

@migueldaipre migueldaipre added the Resolution: PR Submitted A pull request with a fix has been provided. label Nov 8, 2024
@elicwhite
Copy link
Member

The mentioned PR was merged. Comment if that PR didn't fix this issue and we can reopen.

@cortinico cortinico added Resolution: Fixed A PR that fixes this issue has been merged. and removed Resolution: PR Submitted A pull request with a fix has been provided. labels Jan 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue: Author Provided Repro This issue can be reproduced in Snack or an attached project. Platform: iOS iOS applications. Resolution: Fixed A PR that fixes this issue has been merged. Tech: Codegen Related to react-native-codegen Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules)
Projects
None yet
Development

No branches or pull requests

5 participants