THEOplayer / react-native-connectors

THEOplayer React Native connectors
MIT License
3 stars 4 forks source link

fromDictionary functions is mising from the module / adapter target name is not valid (multi-target projects) #74

Open zachariast opened 11 months ago

zachariast commented 11 months ago

First of all, thank you for your amazing and very helpful contribution.

I encountered an issue with the module in my project. It appears that the extension file (youbora/Extensions/YBOptions+fromDictionary.swift), was not included in the module, causing an error when I tried to use it.



DispatchQueue.main.async {
    let theView = self.bridge.uiManager.view(forReactTag: node) as? THEOplayerRCTView
    if let player = theView?.player {
        let options = YBOptions.fromDictionary(youboraOptions)
        let plugin = YBPlugin(options: options)
        plugin.adapter = YBTHEOPlayerAdapter(player: player)
        self.connectors[node] = plugin
    }
}

Current solution:

I've noticed also another point that requires attention in your documentation. In the following section of your docs, you recommend adding a pre-install hook within the Podfile. While this approach works seamlessly for projects with a single target, it presents some challenges for us since we have two targets in our project. Specifically, the adapter_target_name = "YouboraTHEOPlayerAdapter" target name was not valid in our context. To address this issue, we adopted a workaround, which proved effective in our case. Here's what we did:

pre_install do |installer|
  adapter_target_names = ["YouboraTHEOPlayerAdapter-iOS", "YouboraTHEOPlayerAdapter-tvOS"]
  theoplayer_target_names = ["THEOplayerSDK-core-iOS", "THEOplayerSDK-core-tvOS"]

  adapter_target_names.each do |adapter_target_name|
    puts "Patching dependencies of #{adapter_target_name}"

    def target_by_name(installer, name)
      target = installer.pod_targets.find { |t| t.name == name }
      raise "No target '#{name}'" unless target
      target
    end

    adapter_target = target_by_name(installer, adapter_target_name)

    theoplayer_target_names.each do |theoplayer_target_name|
      adapter_target.dependent_targets <<= target_by_name(installer, theoplayer_target_name)
    end
  end
end

This adaptation enabled us to seamlessly handle multiple targets within our project. However, it's important to note that this workaround, while effective for our specific scenario, may not be considered an elegant or best-practice solution. The reason is that it involves manual definition of target names, which could potentially pose challenges for projects with a larger number of targets.

wvanhaevre commented 10 months ago

Thanks for reporting this. I indeed see now that the YBOptions+fromDictionary.swift file is not correctly packaged. The podSpec states s.source_files = "ios/**/*.{h,m,mm,swift}" but that file is located at /Extensions I'll move the swift file inside the ios folder, which should solve the issue.

Thanks also for the suggestion on the pre_install phase. We'll update our docs to make this clear to other users.

wvanhaevre commented 10 months ago

I'm considering to simplify the setup to just:

pre_install do |installer|
  ios_youbora_target = installer.pod_targets.find { |t| t.name == "YouboraTHEOPlayerAdapter-iOS" }
  ios_theoplayer_target = installer.pod_targets.find { |t| t.name == "THEOplayerSDK-core-iOS"}
  puts "Adding THEOplayerSDK-core-iOS as a target dependency to YouboraTHEOPlayerAdapter-iOS"
  ios_youbora_target.dependent_targets <<= ios_theoplayer_target

  tvos_youbora_target = installer.pod_targets.find { |t| t.name == "YouboraTHEOPlayerAdapter-tvOS"}
  tvos_theoplayer_target = installer.pod_targets.find { |t| t.name == "THEOplayerSDK-core-tvOS"}
  puts "Adding THEOplayerSDK-core-tvOS as a target dependency to YouboraTHEOPlayerAdapter-tvOS"
  tvos_youbora_target.dependent_targets <<= tvos_theoplayer_target
end

In addition I will also check for the possibility to add the THEOplayerSDK-core dependency to the YouboraTHEOPlayerAdapter pod. Which makes sense as it depends on the THEOplayerSDK internally. That should drop the requirement to add this script to the podFile.