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

Changing hook type #3

Closed
tabrindle opened this issue Apr 21, 2016 · 14 comments
Closed

Changing hook type #3

tabrindle opened this issue Apr 21, 2016 · 14 comments

Comments

@tabrindle
Copy link

Would there be any adverse or unexpected effects of changing the hook type from after_platform_add to after_plugin_add? We have an app with a sizeable amount of code on the native side and its not practical to generate the ios platform on each checkout.

@akofman
Copy link
Owner

akofman commented Apr 22, 2016

Hello,

Once this plugin installed, it executes a script to check the presence of bridging headers in the other plugins of your project.
In order to do that, it must be installed after all other plugins. That's why the script is executed after the platform is added to be sure all other plugins have already been installed.
Unfortunately, I tried with an after_prepare hook but it doesn't work with plugins, I'll check that in the Cordova codebase because I think it should work like that.

An after_plugin_add hook would be executed once after adding this plugin. So it won't guaranteed the presence of bridging headers in all other plugins if it is added before them.

In your case, I think the best solution at the moment would be to directly add the add-swift-support script as a hook in your project.
Hope it helps.

@tabrindle
Copy link
Author

Thank you very much for the quick response. This is pretty much what I expected, and It seems that calling the script directly works. Thanks!

@becvert
Copy link

becvert commented Jun 8, 2016

Hi @akofman

an after_prepare hook seems to work but only when you give explicitly the platform to the command

cordova prepare doesn't trigger the hook

cordova prepare ios does trigger the hook

does it make sense? is there a way to fix that?

Thank you

@akofman
Copy link
Owner

akofman commented Jun 8, 2016

Hey @becvert ! Very helpful comment. Thanks for that.
I'm gonna test it and keep you informed. If it's fine, I will update the hook and the README of this plugin and will check with the Cordova team if it is an expected behaviour.

Thanks again.

@becvert
Copy link

becvert commented Jun 8, 2016

My guess is that you shoudn't wrap the hook inside the <platform> element

@akofman
Copy link
Owner

akofman commented Jun 8, 2016

Sounds good ! I just have to check if it doesn't throw any error messages in case of a non-ios platform.

akofman added a commit that referenced this issue Jun 8, 2016
@akofman
Copy link
Owner

akofman commented Jun 8, 2016

@becvert it's fine to me. If you could have a try with this last commit and confirm everything is also right to you, I'll merge it.

@becvert
Copy link

becvert commented Jun 9, 2016

It's fine to me too

@akofman
Copy link
Owner

akofman commented Jun 9, 2016

Thanks ! Merged in version 1.1.0.

@akofman akofman closed this as completed Jun 9, 2016
@becvert
Copy link

becvert commented Jun 10, 2016

@akofman
When a plugin dependent of yours is first installed, its header file doesn't get added to the Bridging Header immediately but only on subsequent prepare.

which is probably not a problem if you do
cordova plugin add myplugin
and then
cordova prepare

but in some instances you install plugins from config.xml by calling cordova prepare once. then it fails.
only a subsequent prepare fixes it.
Does it make sense?

I believe now that this hook is better:

<hook type="after_plugin_add" src="src/add-swift-support.js" />

sorry for the confusion...

@akofman
Copy link
Owner

akofman commented Jun 10, 2016

Hmm indeed, I think we could keep both hooks. We'll do that layer.
Le 10 juin 2016 12:27 PM, "becvert" notifications@github.com a écrit :

@akofman https://github.com/akofman
When a plugin dependent of yours is first installed, its header file
doesn't get added to the Bridging Header immediately but only on subsequent
prepare.

which is probably not a problem if you do
cordova plugin add myplugin
and then
cordova prepare

but in some instances you install plugins from config.xml by calling cordova
prepare once. then it fails.
only a subsequent prepare fixes it.
Does it make sense?

I believe now that this hook is better:

sorry for the confusion...


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#3 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAjZUqQeXZK9YExVYkIyFlx5czDtxhvFks5qKTv6gaJpZM4INFRA
.

@becvert
Copy link

becvert commented Jun 10, 2016

that will do 👍
thank you

@akofman
Copy link
Owner

akofman commented Jun 11, 2016

OK from this branch I added some hooks and updated the script.

I tested multiple cases :

  • If the plugin is added first then the platform.
  • If the platform is added first then this plugin is added.
  • If the plugin is configured from the config.xmlfile then the platform is added.
  • If an other Swift plugin bundled with a custom Bridging-Header is added then its Bridging-Header is well merged.
  • Also I've kept the after_prepare hook so that if we are in a case with an already configured platform without the swift support, it will be well added after preparing the platform.

Any objections ?

@becvert
Copy link

becvert commented Jun 13, 2016

none 👍

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

No branches or pull requests

3 participants