search

mrousavy / nitro

🔥 Insanely fast native C++, Swift or Kotlin modules with a statically compiled binding layer to JSI
https://nitro.margelo.com
MIT License
556 stars 9 forks source link

`HybridObject` needs to receive platform arguments #199

Open hannojg opened 2 weeks ago

hannojg commented 2 weeks ago

I was confused as the jsdoc states that the default generic parameter for HybridObject would be:

https://github.com/mrousavy/nitro/blob/2e2e67fbeb96efb4fb8bfa779d6ed009d42675d4/packages/react-native-nitro-modules/src/HybridObject.ts#L24-L35

However, when trying to generate a spec specifying no generic parameter:

export interface PointerHolder extends HybridObject {

i get this error: 

🔧  Loading nitro.json config...
🚀  Nitrogen runs at ~/Documents/Github/react-native-filament/package
    🔍  Nitrogen found 2 specs in ./src/specs
⏳  Parsing PointerHolder.nitro.ts...
⚠️   PointerHolder does not declare any platforms in HybridObject<T> - nothing can be generated.
    ❌  No specs found in PointerHolder.nitro.ts!
mrousavy commented 2 weeks ago

Ah right, there's some pre-parsing code that still validates this. We should just be able to remove that and everything will work.

I'll take a look at this soon, I think for now you're not blocked by just explicitly adding the type arguments right?

hannojg commented 2 weeks ago

No I am not blocked, all good, just wanted to report this for DX

mrousavy commented 1 week ago

Let's turn this into a discussion; do we even want a default-behaviour for this?

Let's say the user doesn't explicitly declare the platforms, so just extends HybridObject. It will generate C++ code on both iOS and Android.

  1. Is that the best behaviour?
  2. Why not Swift and Kotlin?
  3. What happens if a new platform (e.g. windows) gets added? Should it also generate for that?