Exporting constants from iOS TurboModule

[birdofpreyru]

Description

Hi. Not sure is it a bug, an overlook in the current documentation, or me missing something; but I just bumped into the following issue with exporting constants from a TurboModule on iOS.

I had a native module exporting constants as said here (for old arch): it overrode - (NSDictionary *)constantsToExport, and all worked fine. I am migrating it to the new arch, with backward compatibility. When I build it and run in compatibility mode (old arch) - it still works fine; if I build and run for new arch - no constants are exported (I checked, constantsToExport is never called, and on JS side getConstants() returns null). I looked into RN documentation, have not found any mentions of changes in exporting constants for iOS TurboModules, nor in the New Arch sections, nor in Migration to the New Arch sections. I looked into the generated Specs for my TurboModule, it has these both:

- (facebook::react::ModuleConstants<JS::NativeReactNativeStaticServer::Constants::Builder>)constantsToExport;
- (facebook::react::ModuleConstants<JS::NativeReactNativeStaticServer::Constants::Builder>)getConstants;

thus I've tried to override getConstants method instead, and it then works fine for the new arch. But leaves me confused: if we are supposed to override getConstants instead of constantsToExport in the new arch, why the previous constantsToExport is still in specs? If not, why the override is not called - a bug? And why nothing is mentioned in the docs in this regard, am I missing something here?

Version

0.71.2

Output of npx react-native info

N/A

Steps to reproduce

N/A

Snack, code example, screenshot, or link to a repository

N/A

[cortinico]

If not, why the override is not called - a bug? And why nothing is mentioned in the docs in this regard, am I missing something here?

Can you share your spec file? Also a reproducer would help here to debug what's going on

[birdofpreyru]

Hey @cortinico , I just wanna say, I still remember about this issue I reported, but I had no time yet to look into creating a minimal reproducer.

Though, the library I am working on is public, here: https://github.com/birdofpreyru/react-native-static-server, and the piece of code which I was refering to is this one: https://github.com/birdofpreyru/react-native-static-server/blob/3bd511dbb70bb3fc18b7b346af0db9a83c6602f8/ios/ReactNativeStaticServer.mm#L26-L36

- (NSDictionary*) constantsToExport {
  return @{
    @"CRASHED": CRASHED,
    @"LAUNCHED": LAUNCHED,
    @"TERMINATED": TERMINATED
  };
}

- (NSDictionary*) getConstants {
  return [self constantsToExport];
}

as I said, from the docs it sounds like just having constantsToExport() should make it work, but in my experience with the new arch the JS side actually calls getConstants() instead, thus I had to add that method.

[github-actions[bot]]

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

[github-actions[bot]]

This issue was closed because it has been stalled for 7 days with no activity.

[ospfranco]

Codegen doesn't seem to be generating any initialization code, just another method

Here is my spec

import { TurboModuleRegistry, TurboModule } from 'react-native';

export interface Spec extends TurboModule {
  getConstants: () => {
    IOS_DOCUMENT_PATH: string;
    IOS_LIBRARY_PATH: string;
    ANDROID_DATABASE_PATH: string;
    ANDROID_FILES_PATH: string;
    ANDROID_EXTERNAL_FILES_PATH: string;
  };

  install(): boolean;
}

export default TurboModuleRegistry.getEnforcing<Spec>('OPSQLite');

Here is the generated code

/**
 * This code was generated by [react-native-codegen](https://www.npmjs.com/package/react-native-codegen).
 *
 * Do not edit this file as changes may cause incorrect behavior and will be lost
 * once the code is regenerated.
 *
 * @generated by codegen project: GenerateModuleObjCpp
 *
 * We create an umbrella header (and corresponding implementation) here since
 * Cxx compilation in BUCK has a limitation: source-code producing genrule()s
 * must have a single output. More files => more genrule()s => slower builds.
 */

#import "OPSQLiteSpec.h"


namespace facebook {
  namespace react {
    
    static facebook::jsi::Value __hostFunction_NativeOPSQLiteSpecJSI_install(facebook::jsi::Runtime& rt, TurboModule &turboModule, const facebook::jsi::Value* args, size_t count) {
      return static_cast<ObjCTurboModule&>(turboModule).invokeObjCMethod(rt, BooleanKind, "install", @selector(install), args, count);
    }

    static facebook::jsi::Value __hostFunction_NativeOPSQLiteSpecJSI_getConstants(facebook::jsi::Runtime& rt, TurboModule &turboModule, const facebook::jsi::Value* args, size_t count) {
      return static_cast<ObjCTurboModule&>(turboModule).invokeObjCMethod(rt, ObjectKind, "getConstants", @selector(getConstants), args, count);
    }

    NativeOPSQLiteSpecJSI::NativeOPSQLiteSpecJSI(const ObjCTurboModule::InitParams &params)
      : ObjCTurboModule(params) {
        
        methodMap_["install"] = MethodMetadata {0, __hostFunction_NativeOPSQLiteSpecJSI_install};
        
        
        methodMap_["getConstants"] = MethodMetadata {0, __hostFunction_NativeOPSQLiteSpecJSI_getConstants};
        
    }
  } // namespace react
} // namespace facebook

Calling the method works, but constants are not exported on the module:

export const {
  // @ts-expect-error
  IOS_DOCUMENT_PATH,
  // @ts-expect-error
  IOS_LIBRARY_PATH,
  // @ts-expect-error
  ANDROID_DATABASE_PATH,
  // @ts-expect-error
  ANDROID_FILES_PATH,
  // @ts-expect-error
  ANDROID_EXTERNAL_FILES_PATH,
} = NativeOPSQLite;

console.warn(JSON.stringify(NativeOPSQLite)); // outputs empty object (because it only has functions)
console.warn(NativeOPSQLite.getConstants()) // outputs constants

The implementation code:

- (NSDictionary *)constantsToExport {
    NSArray *libraryPaths = NSSearchPathForDirectoriesInDomains(NSLibraryDirectory, NSUserDomainMask, true);
    NSString *libraryPath = [libraryPaths objectAtIndex:0];
    
    NSArray *documentPaths = NSSearchPathForDirectoriesInDomains(NSDocumentDirectory, NSUserDomainMask, true);
    NSString *documentPath = [documentPaths objectAtIndex:0];
    return @{
        @"IOS_DOCUMENT_PATH": documentPath,
        @"IOS_LIBRARY_PATH": libraryPath
    };
}

- (NSDictionary *)getConstants {
    return [self constantsToExport];
}

For now, I've worked around the issue by introducing ts-errors and calling the getConstants method myself:

export const {
  // @ts-expect-error
  IOS_DOCUMENT_PATH,
  // @ts-expect-error
  IOS_LIBRARY_PATH,
  // @ts-expect-error
  ANDROID_DATABASE_PATH,
  // @ts-expect-error
  ANDROID_FILES_PATH,
  // @ts-expect-error
  ANDROID_EXTERNAL_FILES_PATH,
} = !!NativeOPSQLite.getConstants
  ? NativeOPSQLite.getConstants() // If the module is a Turbo Module, it will be there
  : NativeOPSQLite; // Old module simply exports them via `constantsToExport`

[ospfranco]

It seems on Android the generated code is different and getConstants is shared between the old-arch and codegen, but this also means it is not overridable and will not work when creating modules that keep compatibility

The generated stub by Android Studio doesn't help either

All in all, it seems this topic was not properly implemented in codegen or was broken in some of the latest versions. Any guidance is much appreciated

[birdofpreyru]

Can’t keep myself from saying — auto-closing issues by GitHub actions is stupid and harmful — hiding dirt under the carpet doesn’t help solving the issue, if the issue remained open and visible, perhaps it helped to avoid degrading the constant problem even further 🤷‍♂️

[ospfranco]

The amount of rubbish issues is hard to manage in a repo like react-native, I have sympathy for the maintainers. I think I will create a new issue. I managed to get it working on Android by just implementing the getTypedConstants method. But in any case, the documentation is missing. I also found this small disclaimer on the old arch modules page:

[cortinico]

The amount of rubbish issues is hard to manage in a repo like react-native, I have sympathy for the maintainers.

That's the underlying problem @birdofpreyru

If the issue gets stale and closed by the bot, let's open a new one. If an issue has a a linked reproducer, it won't stale. Honestly we receive so many bugs that we can't cope with the traffic so that's a necessary measure.

@ospfranco let's create a new issue for your particular case. We're specifically prioritizing New Architecture bugs as of now, so this would be really precious

[ospfranco]

ok, awesome, will create later

[birdofpreyru]

Hmm... I am working on migrating my up-to-date fork of react-native-fs to RN v0.73; I just got it working for Android with both new and old archs, and I haven't noticed any significant issues with constants so far (beside a lack of documentation in RN web).

On TS side I have the constants method defined as getConstants(). Codegen seems to work fine and for new arch it generates me the specs like:

protected abstract Map<String, Object> getTypedExportedConstants();

@Override
@DoNotStrip
public final @Nullable Map<String, Object> getConstants() {
  Map<String, Object> constants = getTypedExportedConstants();
  // Here codegen adds a bunch of assertion code for debug & internal builds.
  return constants;
}

so I just implement getTypedExportedConstants() function, and for the old arch I have a stub specs file that just does:

  abstract fun getTypedExportedConstants(): Map<String, Object>

  override fun getConstants(): Map<String, Any>? {
    return getTypedExportedConstants()
  }

So far so good, very curious to see how (whether) it will work for me with iOS.

[birdofpreyru]

Yeah, works fine for me with RN v0.73 on iOS, macOS, and Windows as well. I mean, there is no behavior changes from v0.72 to v0.73, only pre-existing issues with logic (which can be worked around) and documentation.

[ospfranco]

I also managed to get it working, there is just a lot of disparity and missing info in the documentation. @cortinico do you still want me to create a new issue?