Fix TODOs in the TurboModule implementation

[kraenhansen]

Umbrella for the TODO/FIXME markers left in the C++ TurboModule implementation.

Note

The permalinks originally in this issue pointed at react-native-node-api-modules and the pre-rename packages/react-native-node-api-modules/cpp/ paths, so none of them resolved any more. Refreshed below against 29a527d, with each marker mapped to the issue that now covers it.

CxxNodeApiHostModule.cpp — requireNodeAddon returns undefined on bad arguments

react-native-node-api/packages/host/cpp/CxxNodeApiHostModule.cpp

Lines 54 to 63 in 29a527d

	CxxNodeApiHostModule::requireNodeAddon(jsi::Runtime &rt,
	react::TurboModule &turboModule,
	const jsi::Value args[], size_t count) {
	auto &thisModule = static_cast<CxxNodeApiHostModule &>(turboModule);
	if (1 == count && args[0].isString()) {
	return thisModule.requireNodeAddon(rt, args[0].asString(rt));
	}
	// TODO: Throw a meaningful error
	return jsi::Value::undefined();
	}

Covered by #50.

CxxNodeApiHostModule.cpp — the addon's declared Node-API version is never read

https://github.com/callstackincubator/react-native-node-api/blob/29a527d87fa8cb965b0048b1b4c1cff2b0eab281/packages/host/cpp/CxxNodeApiHostModule.cpp#L127-L131

https://github.com/callstackincubator/react-native-node-api/blob/29a527d87fa8cb965b0048b1b4c1cff2b0eab281/packages/host/cpp/CxxNodeApiHostModule.cpp#L142-L145

Covered by #4 (sub-issue), which also depends on #3 for the napi_module_register half. Both call sites already carry @see links to #4 in the source.

CxxNodeApiHostModule.hpp + AddonLoaders.hpp — Windows loaders

The LoaderPolicy alias is hardcoded to PosixLoader, so Win32Loader/WinRTLoader are never selected, and all four Windows failure paths detect an error and discard it instead of calling GetLastError():

react-native-node-api/packages/host/cpp/CxxNodeApiHostModule.hpp

Lines 45 to 46 in 29a527d

	using LoaderPolicy = PosixLoader; // FIXME: HACK: This is temporary workaround
	// for my lazyness (work on iOS and Android)

react-native-node-api/packages/host/cpp/AddonLoaders.hpp

Lines 45 to 100 in 29a527d

	#if defined(_WIN32)
	struct Win32Loader {
	using Module = HMODULE;
	using Symbol = void *;
	static Module loadLibrary(const char *filePath) {
	assert(NULL != filePath);
	Module result = LoadLibrary(filePath);
	if (NULL == result) {
	// TODO: Handle the error case... call GetLastError() that gives us error
	// code as DWORD
	}
	return result;
	}
	static Symbol getSymbol(Module library, const char *name) {
	assert(NULL != library);
	assert(NULL != name);
	Symbol result = GetProcAddress(library, name);
	if (NULL == result) {
	// TODO: Handle the error case... call GetLastError() that gives us error
	// code as DWORD
	}
	return result;
	}
	static void unloadLibrary(Module library) {
	if (NULL != library) {
	FreeLibrary(library);
	}
	}
	};
	struct WinRTLoader {
	using Module = HMODULE;
	using Symbol = void *;
	static Module loadLibrary(const char *filePath) {
	assert(NULL != filePath);
	Module result = LoadPackagedLibrary(filePath);
	if (NULL == result) {
	// TODO: Handle the error case... call GetLastError() that gives us error
	// code as DWORD
	}
	return result;
	}
	static Symbol getSymbol(Module library, const char *name) {
	assert(NULL != library);
	assert(NULL != name);
	Symbol result = GetProcAddress(library, name);
	if (NULL == result) {
	// TODO: Handle the error case... call GetLastError() that gives us error
	// code as DWORD
	}
	return result;

Covered by #419.

Markers added since this issue was filed

• HermesNapiHost.cpp — napi_fatal_exception aborts instead of routing through React Native's error handling → Route napi_fatal_exception through React Native's error handling instead of aborting #402
• Logger.cpp — log_debug is not compiled out or gated in release builds → Disable debug logging in release builds #420