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

Come up with an simple action example which uses cocoapods #34

Open
seletz opened this Issue Nov 11, 2016 · 24 comments

Comments

Projects
None yet
3 participants
@seletz
Copy link
Contributor

seletz commented Nov 11, 2016

As discussed in #26 we need some solution for managing and sharing extensions outside of the
main GH repo.

Let's try to get a cocoapods enabled simple example action going.

@seletz

This comment has been minimized.

Copy link
Contributor Author

seletz commented Nov 11, 2016

This issue might be worth noting -- XCode 8 and cocoapods ugliness:

CocoaPods/CocoaPods#5997

@seletz

This comment has been minimized.

Copy link
Contributor Author

seletz commented Nov 11, 2016

@seletz

This comment has been minimized.

@seletz

This comment has been minimized.

Copy link
Contributor Author

seletz commented Nov 18, 2016

So I'm in the process of doing the demo for cocoapods.

Thing is, actions currently have access to the jason client object just because they're in the same project. This is not the case for cocoapods based actions. So I thought about implementing a new type class with prefix '@' which will be for plug-ins. There we can pass the jason object in the plug-in initializer.

However, we still need to create a interface and header file for the Jason object and make that one public by some means.

Another solution would be to have the plug-in NOT know anything about the jason object and have it send messages. We'd need to register observers for those messages. I currently don't know if these messages are handled sync or async. That might complicate stuff.

@seletz

This comment has been minimized.

Copy link
Contributor Author

seletz commented Nov 18, 2016

This is what I have in mind:

seletz/JasonetteDemoAction@d2393e6

@seletz

This comment has been minimized.

Copy link
Contributor Author

seletz commented Nov 18, 2016

This would be in the core -- the notification handlers are still missing:
nexiles@c3b1850

@seletz

This comment has been minimized.

Copy link
Contributor Author

seletz commented Nov 18, 2016

Actually, thinking a bit further: If PlugIns would be Singletons, they could add themselves to the default notification center. So we could do away with the rather ugly and unsafe respondsToSelector: and performSelector: calls. We'd just use the type string of the action as notification name.

That would allow for a real decoupled implementation.

@seletz

This comment has been minimized.

Copy link
Contributor Author

seletz commented Nov 18, 2016

@gliechtenstein

This comment has been minimized.

Copy link
Contributor

gliechtenstein commented Nov 18, 2016

Another idea I could think of was using delegates. Probably works similar to your first idea. I think the (speculative) benefit is that it may be more efficient than broadcasting notifications, but the difference may not be that significant, so this argument really depends on whether nsnotificationcenter is actually faster than delegates in a palpable way.

That said, I like the decoupled nature of the notification based solution. Notifications make it much easier to implement cross-thread action calls (this may solve some existing problems with other actions, such as $timer, which is super inefficient right now), and potentially could become the seed to implement multithreaded action call chains (right now for simplicity's sake there can be only a single action call chain happening at any given point)

In fact we could even potentially rewrite all actions this way as long as the performance difference is not that big of a deal. That would enable us to move the init method into JasonAction so we have minimal amount of code in the extension code.

So I guess my only question is the performance. Do you have any experience with this?

@seletz

This comment has been minimized.

Copy link
Contributor Author

seletz commented Nov 18, 2016

Well, iOS heavily uses notifications all the time. So IMHO performance is a non-issue.

I also agree that all actions could be implemented that way.

Also, I think the JASON core object should register responders, and thus we have a clean separation between internal API and external/plug in API.

@gliechtenstein

This comment has been minimized.

Copy link
Contributor

gliechtenstein commented Nov 18, 2016

I think the JASON core object should register responders, and thus we have a clean separation between internal API and external/plug in API.

Could you elaborate on this? Register responders for what?

@seletz

This comment has been minimized.

Copy link
Contributor Author

seletz commented Nov 18, 2016

Of course, there is a drawback also: Using strings and dictionaries with string keys is prone to typing errors. But Jasonette is about JSON, so ...

@seletz

This comment has been minimized.

Copy link
Contributor Author

seletz commented Nov 18, 2016

Register reponders for all the things a plug-in might want to call.

@gliechtenstein

This comment has been minimized.

Copy link
Contributor

gliechtenstein commented Nov 18, 2016

@seletz yeah i'm not so worried about the type related issues, as long as we stick to nsdictionary and follow convention it should be fine. As for the responders you mean "success" and "error" callbacks?

p.s. I think we could also abstract the actual NSNotificationCenter postnotification part into JasonAction and call that instead of the current [[Jason client] success: ...]

@seletz

This comment has been minimized.

Copy link
Contributor Author

seletz commented Nov 18, 2016

Proof Of Concept Code

Example Plugin code is here:
https://github.com/seletz/JasonetteDemoAction/blob/develop/JasonetteDemoAction/Classes/JasonetteDemoAction.m

Feature branch on Jasonette core to get this going is here:

Example jason file is here:

https://jasonbase.com/things/zEQ

Podfile changes to add the module is here:

https://github.com/nexiles/JASONETTE-iOS/blob/feature/plugin-module-handling/app/Podfile#L37

How does it work?

User

A user just needs to do a simple:

$ pod install JasonetteDemoAction

And then recompile. (note this will work when podspecs are pushed to cocoapods trunk)

Plugins then can be used like so:

{
  "$jason": {
    "head": {
      "title": "Jasonette Demo Action",
      "actions": {
        "$load": {
          "type": "@JasonetteDemoAction.demo",
          "options": {
            "text": "foo"
          }
        }
      }
    },
    "body": {
      "header": {
        "title": "Jasonette Demo Action"
      },
      "sections": {
        "items": [
          {
            "type": "label",
            "text": "???"
          }
        ]
      }
    }
  }
}

Developer

A plugin developer needs to create a class with an initializer. This class must register notification
handlers for the API the plugin wishes to offer. The action class also may send notifications to the Jasonette app.

@gliechtenstein

This comment has been minimized.

Copy link
Contributor

gliechtenstein commented Nov 18, 2016

Will need some time to process since I'm outside and looking at this through the phone, but at first sight looks like it should work fine!

I initially only thought about the callbacks being notification based but looks like we're going all in with notifications here and even the actual action call is initiated by notifications. Each action is like a client-side microservices function :)

I'll try this out and share my thoughts soon!

@seletz

This comment has been minimized.

Copy link
Contributor Author

seletz commented Nov 18, 2016

@gliechtenstein cool. Yes, the microservice thing was the basic idea. We still need to think about the actual run queue the notifications are called with, and whether or not async action responses pose a problem.

@seletz

This comment has been minimized.

Copy link
Contributor Author

seletz commented Nov 18, 2016

BTW, it should be possible to code plug-ins using swift using that approach. Swift always compiles in the module name, and the syntax would be @ModuleName.PluginClass.action.

@gliechtenstein

This comment has been minimized.

Copy link
Contributor

gliechtenstein commented Nov 19, 2016

@seletz I think we're good to go! I spent some time trying to come up with a way to refactor the entire action mechanism but realized that may be a bit too much and might be too risky at the moment. So I think the way we have it here is great. Can you send a pull request?

@gliechtenstein

This comment has been minimized.

Copy link
Contributor

gliechtenstein commented Nov 21, 2016

We still need to think about the actual run queue the notifications are called with, and whether or not async action responses pose a problem.

I do think this is important, but since it looks like this wouldn't affect the main code unless people use the @ notation, it would be OK to just merge it and fix as we discover cases that don't work? Do you see any potential problem it can cause to normal usage?

@seletz

This comment has been minimized.

Copy link
Contributor Author

seletz commented Nov 21, 2016

Yep, I think merging this would be OK. I just realised that I actually think I need to open e new PR for the plug-in handling. 😄

@seletz

This comment has been minimized.

Copy link
Contributor Author

seletz commented Nov 21, 2016

@cpg

This comment has been minimized.

Copy link
Contributor

cpg commented Nov 25, 2016

Hi, I don't follow exactly what's going on here, but it looks like this is a demo of a plugin that has an action?

If it's actually a "demo," does it really need to be in the core?

I pulled and merged from develop and for some reason, even though it's in cocoapods, it's preventing me from compiling Jasonette:

bash$ gem install cocoapods --version 1.2.0.beta.1
Fetching: cocoapods-core-1.2.0.beta.1.gem (100%)
Successfully installed cocoapods-core-1.2.0.beta.1
Fetching: ruby-macho-0.2.6.gem (100%)
Successfully installed ruby-macho-0.2.6
Fetching: cocoapods-1.2.0.beta.1.gem (100%)
Successfully installed cocoapods-1.2.0.beta.1
3 gems installed
bash$ pod install
Analyzing dependencies
[!] Unable to find a specification for `JasonetteDemoAction (~> 0.1)`
bash$ pod install JasonetteDemoAction
[!] Unknown command: `JasonetteDemoAction`

Usage:
.....
@gliechtenstein

This comment has been minimized.

Copy link
Contributor

gliechtenstein commented Nov 25, 2016

@cpg First try the approaches mentioned here: #62

As for why this was pulled in, I thought it would be nice to have a single demo Pod in the project so people can take a look and follow by adding their own.

That said, this problem happened to me too, and I thought this was just me, but if this is happening to more people we should take this out. Will investigate and update here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.