Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

iOS bindings #7

Merged
merged 3 commits into from
Nov 22, 2022
Merged

iOS bindings #7

merged 3 commits into from
Nov 22, 2022

Conversation

kaloudis
Copy link
Contributor

@kaloudis kaloudis commented Nov 17, 2022

This PR includes the iOS bindings for lnc-rn and update to the scaffolding of the demo apps.

Git LFS has been added to handle the iOS binaries for LNC mobile

Copy link
Member

@jamaljsr jamaljsr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got the connect-demo working in the iOS simulator with the following changes. I suggest updating this PR with these changes before merging.

It would also be really great to provide instructions on what commands to use to run the demo apps as I had to fumble my way through it.

  1. Replaced @import Foundation with #import <Foundation/Foundation.h> in Mobile.objc.h and Universe.objc.h to resolve the build error
/lnc-rn/ios/Lndmobile.xcframework/ios-arm64/Lndmobile.framework/Headers/Universe.objc.h:9:1: error: use of '@import' when C++ modules are disabled, consider using -fmodules and -fcxx-modules
  1. Ran the following commands to install the lnc-rn package using a relative path in package.json
cd demos/connect-demo
npm uninstall @lightninglabs/lnc-rn
npm install ../..
  1. Updated the demos/connect-demo/metro.config.js to remove the hard-coded path
const path = require('path');
const { getDefaultConfig } = require('metro-config');

module.exports = (async () => {
  const {
    resolver: { sourceExts, assetExts }
  } = await getDefaultConfig();
  return {
    watchFolders: [
      path.resolve(path.join(__dirname, '..', '..'))
    ],
    resolver: {
      assetExts: assetExts.filter(ext => ext !== 'svg'),
      sourceExts: [...sourceExts, 'svg']
    }
  };
})();

@kaloudis
Copy link
Contributor Author

@jamaljsr 2+3 should be resolved when we publish the package and update the demo apps to point to the version on npm

For 1, I think we should fix the import before uploading the library files on GitHub. For those who want to build the libraries from scratch, we can include that step in the Install instructions. Working on some updates over at #9

@jamaljsr
Copy link
Member

@jamaljsr 2+3 should be resolved when we publish the package and update the demo apps to point to the version on npm

Ok, but it would make my life easier when switching branches to review PRs 😊. If you don't expect there to be any more PRs before release, then it's fine to leave it as is.

For 1, I think we should fix the import before uploading the library files on GitHub. For those who want to build the libraries from scratch, we can include that step in the Install instructions. Working on some updates over at #9

Is this something that will always be necessary? If so, it makes sense to automate the string replacement via a make/bash script when running make ios.

@kaloudis
Copy link
Contributor Author

Automated the iOS string replacement here: lightninglabs/lightning-node-connect#63

@kaloudis kaloudis merged commit 957b9c4 into main Nov 22, 2022
@kaloudis kaloudis deleted the ios branch November 22, 2022 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants