Proposal: add a way for packages to communicate with one another

[benogle]

I added an API to the status-bar. Other packages that want to use the status bar have a bit of an awkward time getting a handle on the statusBar object. For reference see: atom/grammar-selector#2 Here's how it works now:

In the status bar, I do this:

statusBar = new StatusBarView()
atom.rootView.statusBar = statusBar

And in the package using status-bar:

# This is dumb:
createStatusEntry = ->
  view = new GrammarStatusView(atom.rootView.statusBar)
  atom.rootView.statusBar.appendLeft(view)

if atom.rootView.statusBar
  createStatusEntry()
else
  atom.packages.once 'activated', ->
    createStatusEntry()

I dont want to have every package author that wants to use something to the status bar to have to do this.

Proposal

I want to have some mechanism in one of the readily available objects (atom, rootView) for storage/retrieval of arbitrary objects. This is generally what I want to do:

In status-bar

statusBar = new StatusBarView()
atom.rootView.setChild('statusBar', statusBar)

In the package that uses status-bar

atom.rootView.getChild 'statusBar', (statusBar) ->
  view = new GrammarStatusView(atom.rootView.statusBar)
  atom.rootView.statusBar.appendLeft(view)

If statusBar is already there, it calls the callback, or it waits til statusBar is available.

I have no idea what to call these functions, and have no preference which object it should hang from.

Feedback, please.

[mcolyer]

One tricky thing here is that our package API specifies that you return an object with functions: activate, deactivate etc rather than a class. So maybe we change that to be a class instead with those instance methods and the package loader instantiates that class when activating a package.

And then maybe we add something like this (which returns that class) and which I think reads a little bit better:

atom.packages.request('status-bar')

My thinking with adding a method onto the package manager is that we could trigger an activate if the package wasn't loaded immediately (ie it's activated on a keystroke).

Thoughts?

[benogle]

I dont understand this:

One tricky thing here is that our package API specifies that you return an object with functions: activate, deactivate etc rather than a class.

Are you saying in my solution? Or the atom.packages.request solution?

How would I get the actual status bar object to interact with in the atom.packages.request solution?

I kind of wanted to shy away from having any indicator that you were interacting with another package. Then it doesnt matter what the package is called (it can change?) or how it's constructed. All I care about is getting the exposed object(s). And if I'm going to directly interact with an object, I'd like to leave it up to the package specify which object it exposes. So what if a package wanted to add a foo and a bar object in there? Would I have to do atom.packages.request('pkg').foo and atom.packages.request('pkg').bar?

That said, @probablycorey and I just talked about having some kind of evented system to have packages talk to each other. But I have to think it through. I dont know what it would look like. Might require complicated stuff to account for when events were sent and the package was not loaded yet. He and @kevinsawicki talked about this kind of thing a while back, and they liked a more evented approach. I have to get more details, though.

Also came up with better names for my proposed solution:

atom.setAttribute()
atom.getAttribute()

[benogle]

Also if atom.packages.request('status-bar') triggers an event if not loaded yet, we have the exact same code I'm using now to get the object: Check for existence, if not there, subscribe to some event. IMO, it should be in one call.

[mcolyer]

Are you saying in my solution? Or the atom.packages.request solution?

I was thinking in either case. Instead of each package having to know to attach something via atom.setAttribute that we automatically register a package singleton in atom.packages for each package. The package could then set attributes on that singleton as it saw fit without having to worry about collisions.

Would I have to do atom.packages.request('pkg').foo and atom.packages.request('pkg').bar?

Yep that's what I was thinking, otherwise I think conflicts can become an issue and it can become difficult to find where things are coming from.

I would probably write it like this statusBar = atom.packages.request('status-bar'); statusBar.view; statusBar.model;

That said, @probablycorey and I just talked about having some kind of evented system to have packages talk to > each other. But I have to think it through. I dont know what it would look like. Might require complicated stuff to > > account for when events were sent and the package was not loaded yet. He and @kevinsawicki talked about this > kind of thing a while back, and they liked a more evented approach. I have to get more details, though.

Originally I was thinking (but forgot to mention) that atom.packages.request('status-bar') which would load the package (if it wasn't already loaded) before returning so it should work as you expected.

He and @kevinsawicki talked about this kind of thing a while back, and they liked a more evented approach. I have to get more details, though.

I think I know what @probablycorey and @kevinsawicki are talking about and I'm totally open to doing it that way as well.

[benogle]

I was thinking in either case. Instead of each package having to know to attach something via atom.setAttribute that we automatically register a package singleton in atom.packages for each package. The package could then set attributes on that singleton as it saw fit without having to worry about collisions.

You'll have to walk me through what that looks like. Maybe with the status bar code.

I still like the setAttribute (or maybe exportAttribute) approach better as the package explicitly exports objects, and the objects look like a builtin, without having to know which package they came from.

[nathansobo]

I really wish I had the workspace and pane rewrite completed. Telepath model subclasses all provide a method called ::behavior that returns a reactive behavior object. Behaviors emit their current value when someone subscribes, then emit future values whenever the value of the underlying property changes.

@subscribe workspace.behavior('statusBar').onValue (statusBar) ->  statusBar?.appendLeft(view)

That's a bit long. We could set up the behavior method to auto-subscribe if you pass a callback as the second argument:

@subscribe workspace.behavior 'statusBar', (statusBar) ->  statusBar?.appendLeft(view)

Basically, reacting to changing values is a fundamental concept that I want to bake into all our model objects. That's part of why I'm working so hard to get us subclassing telepath.Model.

[nathansobo]

I forgot to add that for declared properties, it's even shorter. All official properties of a model get matching $-prefixed properties for accessing reactive behaviors based on them. But it's tricky here because statusBar will be added by a package, but if we actually declared it then we'd be able to write:

@subscribe workspace.$statusBar.onValue (statusBar) -> ...

[benogle]

Interesting. I like it. So can status-bar package just do a workspace.statusBar = new StatusBar() and it fires those callbacks?

This can wait til you get the rewrite finished. Def not pressing for right now. No one is going to use the status bar api right now.

[nathansobo]

As far as what @mcolyer is saying regarding the package singleton, there's absolutely no reason why the top-level package module couldn't be used as that singleton object. In fact I'd argue that makes the most sense. Then the top level module becomes a kind of global interface to that package.

Wouldn't we want request to be async though?

@subscribe atom.packages.request 'status-bar', (statusBar) ->
  # this is the main module, so it also has activate, serialize, deactivate on it
  # but it has custom methods for other users to interact with as well...
  statusBar.appendLeft(view)

This only works of course when you actually have a singleton, like we will with the global status bar. Otherwise using stock reactive primitives against the model makes more sense.

[nathansobo]

@benogle The only caveat is that you have to explicitly declare properties. Now that I think of it, it may be possible to use proxy objects to get around that. So for now the example would have to look like this, sadly:

workspace.set('statusBar', new StatusBar)

It would be pretty easy to add an instance method for declaring one-off properties, like this:

workspace.property('statusBar')
workspace.statusBar = new StatusBar

But until this property-defining code runs, you're still stuck using the higher-level getters. I guess we could make ::property idempotent so you could call it in the client code just in case it isn't already defined:

workspace.property('statusBar')
@subscribe workspace.$statusBar.onValue (statusBar) ->  # ...

[mcolyer]

Yeah I think the reactive approach would serve well here and I think that would achieve what @benogle is looking to do. I'm also open to making it async as @nathansobo suggested but I get the sense from @benogle that he'd rather not deal with async in the packages for this, so maybe the reactive approach is best here.

I'd still like to advocate for not allowing packages to choose to overwrite any property they see fit on atom.packages and instead registry a property for the name of the package when a package is loaded (but not activated)?

[nathansobo]

Yeah. I think each package's top-level module should basically get registered somewhere automatically. I don't think authors should be encouraged to assign properties into a registry, but instead expose an interface via their top level module for other people to use.

It would be really nice to be able to say atom.packages.statusBar but I see how that puts us in contention a little bit with the genuine properties on the object. How about atom.packages.get('statusBar') just returning the top level module? It would blend nicely with atom.packages.request 'statusBar' -> # ....

I was only thinking async because if the package isn't loaded yet, we may have some work to do? I guess the question is this: when you're requesting, do you explicitly want the package to be loaded or do you just want your thing to be called in the event that it is? Maybe there's 2 methods? One that explicitly triggers a load and one that just watches for one in case it happens? They could be called like: load and onLoad or something?

[mcolyer]

@nathansobo 👍

I like atom.packages.get('statusBar') and atom.packages.request 'statusBar' -> # .... I think the get interface is easier to deal with/reason about and just have that block to activate the package if it isn't already loaded.

[nathansobo]

Bringing this back from the dead. Users are asking for a feature like this. Upon further reflection my plan is to implement @mcolyer's request method, but always do it async. That gives us a single consistent API that either runs your callback immediately if the packages is active or defers it until the package is activated later. I'll call the callback with the requested package's top-level module.

Let me know if you have any feedback. I'll also link the PR here when it's ready.

[kevinsawicki]

Should this be promise-based to be consistent with atom.packages.activatePackage?