Skip to content

Commit

Permalink
Make C++ struct generator type-safe (#35656)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #35656

Changelog: [Internal]

## Change:

#35265 added a struct generator - but it is not type safe :-(

E.g. you can write a TM Spec type as:

```
export type CustomType = {
   key: string;
   enabled: boolean;
   time?: number;
 }
```
And a C++ type as:
```
using CustomType = NativeSampleModuleBaseCustomType<float, bool, std::optional<int32_t>>;
template <>
struct Bridging<CustomType>
    : NativeSampleModuleBaseCustomTypeBridging<float, bool, std::optional<int32_t>> {};
```
and it will still compile :-( - which should not.

The reason is that the generated structs don't validate the members type :-(
```
template <typename P0, typename P1, typename P2>
struct NativeSampleModuleBaseCustomType {
  P0 key;
  P1 enabled;
  P2 time;
  bool operator==(const NativeSampleModuleBaseCustomType &other) const {
    return key == other.key && enabled == other.enabled && time == other.time;
  }
};

template <typename P0, typename P1, typename P2>
struct NativeSampleModuleBaseCustomTypeBridging {
  static NativeSampleModuleBaseCustomType<P0, P1, P2> fromJs(
      jsi::Runtime &rt,
      const jsi::Object &value,
      const std::shared_ptr<CallInvoker> &jsInvoker) {
    NativeSampleModuleBaseCustomType<P0, P1, P2> result{
      bridging::fromJs<P0>(rt, value.getProperty(rt, "key"), jsInvoker),
      bridging::fromJs<P1>(rt, value.getProperty(rt, "enabled"), jsInvoker),
      bridging::fromJs<P2>(rt, value.getProperty(rt, "time"), jsInvoker)};
    return result;
  }

  static jsi::Object toJs(
      jsi::Runtime &rt,
      const NativeSampleModuleBaseCustomType<P0, P1, P2> &value) {
    auto result = facebook::jsi::Object(rt);
    result.setProperty(rt, "key", bridging::toJs(rt, value.key));
    result.setProperty(rt, "enabled", bridging::toJs(rt, value.enabled));
    if (value.time) {
      result.setProperty(rt, "time", bridging::toJs(rt, value.time.value()));
    }
    keyToJs(rt, value.key);
    return result;
  }
};
```

This fixes that, by simply emitting conversion functions for each member such as
```
#ifdef DEBUG
  static bool keyToJs(jsi::Runtime &rt, P0 value) {
    return bridging::toJs(rt, value);
  }
  static double enabledToJs(jsi::Runtime &rt, P1 value) {
    return bridging::toJs(rt, value);
  }
  static jsi::String timeToJs(jsi::Runtime &rt, P2 value) {
    return bridging::toJs(rt, value);
  }
#endif
```

Reviewed By: cipolleschi

Differential Revision: D42082423

fbshipit-source-id: 5133f14e2aa8351e9bbbf614117a3d5894b17fa6
  • Loading branch information
christophpurrer authored and facebook-github-bot committed Dec 16, 2022
1 parent b2dbfbc commit c7e1e00
Show file tree
Hide file tree
Showing 2 changed files with 257 additions and 110 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,20 @@ function createStructs(
const templateParameter = value.properties
.map((v, i) => 'P' + i)
.join(', ');
const paramemterConversion = value.properties
.map((v, i) => {
const translatedParam = translatePrimitiveJSTypeToCpp(
v.typeAnnotation,
false,
typeName =>
`Unsupported type for param "${v.name}". Found: ${typeName}`,
resolveAlias,
);
return ` static ${translatedParam} ${v.name}ToJs(jsi::Runtime &rt, P${i} value) {
return bridging::toJs(rt, value);
}`;
})
.join('\n');
return `#pragma mark - ${structName}
template <${templateParameterWithTypename}>
Expand Down Expand Up @@ -238,25 +252,29 @@ ${value.properties
return result;
}
#ifdef DEBUG
${paramemterConversion}
#endif
static jsi::Object toJs(
jsi::Runtime &rt,
const ${structName}<${templateParameter}> &value,
const std::shared_ptr<CallInvoker> &jsInvoker) {
auto result = facebook::jsi::Object(rt);
${value.properties
.map((v, i) => {
if (v.optional) {
return ` if (value.${v.name}) {
result.setProperty(rt, "${v.name}", bridging::toJs(rt, value.${v.name}.value(), jsInvoker));
}`;
} else {
return ` result.setProperty(rt, "${v.name}", bridging::toJs(rt, value.${v.name}, jsInvoker));`;
}
})
.join('\n')}
return result;
}
};
jsi::Runtime &rt,
const ${structName}<${templateParameter}> &value,
const std::shared_ptr<CallInvoker> &jsInvoker) {
auto result = facebook::jsi::Object(rt);
${value.properties
.map((v, i) => {
if (v.optional) {
return ` if (value.${v.name}) {
result.setProperty(rt, "${v.name}", bridging::toJs(rt, value.${v.name}.value(), jsInvoker));
}`;
} else {
return ` result.setProperty(rt, "${v.name}", bridging::toJs(rt, value.${v.name}, jsInvoker));`;
}
})
.join('\n')}
return result;
}
};
`;
})
Expand Down
Loading

0 comments on commit c7e1e00

Please sign in to comment.