Skip to content

feat: Add Objective-C source code to .pbxproject #3524

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

Merged
merged 13 commits into from
May 25, 2018

Conversation

tdermendjiev
Copy link
Contributor

PR Checklist

What is the current behavior?

Currently, support for native Objective-C source code is not implemented.

What is the new behavior?

Following changes allow plugin authors to include raw Objective-C source code (.m and .h files) to their plugins. CLI looks for folder named platforms/ios/src folder and creates a pbxGroup in the .pbxProject named after the plugin. Files and folders are added to project recursively (NativeScript/node-xcode#6) and same structure is kept. Root folder (platforms/ios/src) is added to HEADER_SEARCH_PATHS as the plugin author still has to provide a modulemap for the source code.

Implements NativeScript/ios-jsc#885.

@tdermendjiev tdermendjiev force-pushed the tdermendzhiev/objc-source-support branch from 97cb2dd to fdfef9d Compare April 10, 2018 11:48
Search for plugin's platforms/ios/src folder and add it and it's content (.h and .m files) to the native iOS project.
@tdermendjiev tdermendjiev force-pushed the tdermendzhiev/objc-source-support branch from fdfef9d to de78502 Compare April 10, 2018 11:59
if (this.$fs.exists(rootPath) && !this.$fs.isEmptyDir(rootPath)) {
this.$fs.readDirectory(rootPath).forEach(fileName => {
const filePath = path.join(rootGroup.path, fileName);
const file: INativeSourceCodeDescription = { name: fileName, path: filePath};
Copy link
Contributor

Choose a reason for hiding this comment

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

name property doesn't seem to be used and might be removed.

@KristianDD
Copy link
Contributor

Try making the paths inside the groups relative, because this might cause problems in the future.

@mbektchiev mbektchiev force-pushed the tdermendzhiev/objc-source-support branch 2 times, most recently from 359b337 to 69321d0 Compare May 16, 2018 15:51
@mbektchiev mbektchiev changed the title Tdermendzhiev/objc source support feat: Add Objective-C source code to .pbxproject May 16, 2018
@mbektchiev
Copy link
Contributor

run ci

1 similar comment
@rosen-vladimirov
Copy link
Contributor

run ci

@mbektchiev mbektchiev force-pushed the tdermendzhiev/objc-source-support branch from 69321d0 to 939afd5 Compare May 17, 2018 11:57
* Implement support for adding resources
* Write unit tests for `Resources` and `src`
* TODO: fix node-xcode reference in package.json after merging Teo's PR
@mbektchiev mbektchiev force-pushed the tdermendzhiev/objc-source-support branch from 939afd5 to 9a33b41 Compare May 17, 2018 14:54
@tdermendjiev
Copy link
Contributor Author

run ci

3 similar comments
@Natalia-Hristova
Copy link

run ci

@Natalia-Hristova
Copy link

run ci

@tdermendjiev
Copy link
Contributor Author

run ci

@tdermendjiev tdermendjiev force-pushed the tdermendzhiev/objc-source-support branch from b8899a6 to 5fd9448 Compare May 22, 2018 13:33
@tdermendjiev tdermendjiev force-pushed the tdermendzhiev/objc-source-support branch from 7de8908 to 6fdc25c Compare May 23, 2018 13:49
@Natalia-Hristova
Copy link

run ci

@Fatme Fatme added this to the 4.1.0 milestone May 25, 2018
Copy link
Contributor

@rosen-vladimirov rosen-vladimirov left a comment

Choose a reason for hiding this comment

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

Great work

@tdermendjiev tdermendjiev merged commit 9b35c64 into master May 25, 2018
@tdermendjiev tdermendjiev deleted the tdermendzhiev/objc-source-support branch May 25, 2018 14:58
@farfromrefug
Copy link
Contributor

@tdermendjiev where is the modulemap supposed to be? i cant get it to work right now inside a plugin. I want to add a NSDate category
Screen Shot 2019-09-17 at 11 43 23
This is what i have.
The content of the modulemap is:

framework module BackgroundGPS {
  umbrella header "BackgroundGPS.h"

  export *
  module * { export * }
}

@mbektchiev
Copy link
Contributor

That should be the right place. Can you try the troubleshooting ideas listed here and see if anything will come up?

@farfromrefug
Copy link
Contributor

@mbektchiev nice article! Did not find the reason of my issue there. But found it it a component i had already doing that. Just didn't remember it :s The reason was the extra "framework" word in the modulemap.
Thanks again @mbektchiev

@mbektchiev
Copy link
Contributor

Seems legit -- a framework module should expect the structure of a .framework bundle: https://clang.llvm.org/docs/Modules.html#module-declaration.

Weren't there any hints about this in the stderr logs of metadata generator?

@farfromrefug
Copy link
Contributor

@mbektchiev not that i saw. The file was 3Mo though so not easy to read. But searching for my module name BackgroundGPS did not give anything.
But dont worry the issue was more on my side. What could be added to the doc though would be a an example on how to add sources. Something with a basic hello word showing the directory structure and the content of the modulemap. I think this would be helpful to many plugin devs

@mbektchiev
Copy link
Contributor

In fact, we have such an example. But maybe it should be more visible. https://github.com/NativeScript/plugin-ios-modulemap-sample

It's linked here: https://docs.nativescript.org/plugins/Use-Native-iOS-Libraries#static-frameworks but obviously not the place that one would look for it ...

@farfromrefug
Copy link
Contributor

Yes would be best for it to be more visible.
And also it does not show how to do it witch custom .m/.h/.c files. It uses Pods. Would be good to add an example with custom sources.
For sure know i will keep an eye on your "docs" repo. Find it easier to navigate than the website.

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.

7 participants