iTwin / itwinjs-core

Monorepo for iTwin.js Library
https://www.itwinjs.org
MIT License
615 stars 210 forks source link

Review core-mobile registering some rpcs by default #5225

Closed aruniverse closed 1 year ago

aruniverse commented 1 year ago
          tangential; i find it weird we have default rpcs to register on Mobile but not on any other platform afaik

cc @itwin/mobilesdkadmins

_Originally posted by @aruniverse in https://github.com/iTwin/itwinjs-core/pull/5103#discussion_r1131113523_

aruniverse commented 1 year ago

Also see https://github.com/iTwin/itwinjs-core/issues/5129 wrt PresentationRPCs

tcobbs-bentley commented 1 year ago

@aruniverse: Regarding mobile vs other platforms, I wasn't aware that it wasn't required on other platforms. I wonder if it is actually required on mobile. Our sample app's backend calls MobileRpcManager.initializeImpl() with an array of supported RPCs. Does that happen automatically for the default set of RPCs? Our frontend doesn't call MobileRpcManager.initializeClient(), and yet the RPCs all work.

Having said that, PresentationRpcInterface is already in the list of RPCs that our sample app's backend passes to MobileRpcManager.initializeImpl(), so if mobile is required to initialize all RPCs, then the change to remove the Presentation dependency in iTwin won't affect us. If we're only supposed to register the RPCs that haven't already been registered, then we can update our sample app to do ath.