atom / apm

Atom Package Manager
https://atom.io/packages
MIT License
1.27k stars 298 forks source link

Packages depending on other (Atom) packages #288

Open travs opened 9 years ago

travs commented 9 years ago

I'll raise this here, but if another repo is more appropriate please redirect me.

Note that there is some interest in having the ability to have Atom packages depend on other Atom packages, and not just NPM packages.

I have created a NPM package to do this, but it is roundabout, and would be much better if it was built into Atom. All it basically does is gives package creators an easy way to force installation of a particular APM package and the ability to access functions from other APM packages' exports.

This idea may not have been taken into account when designing packages originally, since all user APM packages are installed to .atom/packages, which would result in version conflicts if two packages require different versions of the same APM package (they can't both be in the dir can they).

Just an idea, and I know that a lot of the Atom packages are user-facing, so requireing them may seem silly, but the ability for each Atom package to expose an API that other Atom packages can build on seems like a strong enhancement to the current system.

I'd like to help with this too, or whatever it turns into, so get in touch! :email:

thedaniel commented 9 years ago

This is something that I think @atom/core wants, but isn't currently working on. There's more discussion on #120 and atom/atom#2412 - I haven't been working on it directly myself but I think the TL;DR is that it will be based on / inspired by service dependencies which are discussed in atom/atom#5165 and now in core to some extent.

nathansobo commented 9 years ago

I actually think that packages shouldn't be depending on other packages, and instead only depending on other services. That allows any package to step in and fulfill a particular service and avoids calcification in the package-dependency graph. However, I do think it's important that we do a better job communicating etiquette around service namespacing if we want to pull this off.

travs commented 9 years ago

@nathansobo I can see where you're coming from with that, and I agree with having clear guidelines for namespacing.

Where is the main discussion on services taking place? I checked out atom/service-hub but there doesn't seem to be much going on in the issues there.

zephraph commented 9 years ago

What's the distribution method for a service? Also, is there any documentation on services besides the uses text detailed in the readme on atom/service-hub?

I completely agree with you @nathansobo, packages shouldn't be a dependency, but I don't think people even know that services are an option. I honestly didn't. As a matter of fact, I didn't even know what service-hub is until just now. I still don't know where/how it's used (or if it even is used).

nathansobo commented 9 years ago

We've had a blog post that's been on hold for weeks due to a series of minor fixes. @jlord @maxbrunsfeld are we ready to publish that post now? Would be nice to add a section to the documentation as well, but we can probably crowdsource that once the article is out.

jlord commented 9 years ago

@nathansobo I've got a tracking issue for all the little parts here: https://github.com/atom/atom/issues/5726.

Most everything is currently in place, however, we needed to remove the deprecation call for the sake of the tests not freaking out, so the last step is to add it back in after Atom 0.188.0 ships.

The blog post had been on hold so that all this was finalized first and code snippets and links matched up, but I think actually at this point we're good enough to go :+1:

nathansobo commented 9 years ago

Thanks @jlord. I say we wait for 0.188.0 so we are sure to pick the most recent version of a given service.

zephraph commented 9 years ago

Cool, thanks folks. I'm really looking forward to this!

capaj commented 9 years ago

@nathansobo obvious example where package dependency is needed are all autocomplete-plus extension packages, which all depend on autocomplete-plus. Autocomplete plus should be merged into core soon, but I think we still need a possibility to extend other packages. Service hub looks nice, but it doesn't solve a use case when I see a package from someone else and want to tweak it, add a feature to it. There is certainly a usecase here which you can't do with service-hub or forking original package.

maxbrunsfeld commented 9 years ago

@capaj I think the autocomplete-plus use case is addressed very well by the services API. Can you describe what you're trying to do in a little more detail, and explain why it couldn't be achieved with services?

oclbdk commented 9 years ago

How about for sharing UI components?

We have a set of Atom-specific React components that other packages depend on (e.g. text input and combobox components that render to elements like mini <atom-text-editor> to maintain consistent styling). If those depend on other UI components in turn, then the tests become a maintenance nightmare -- since tests don't activate any packages by default, we need to manually write atom.packages.activatePackage() for each recursive package dependency so their services are available. Also, it makes it easy for users to end up in a state with unsatisfied dependencies.

I drafted a proposal for the services API to support that use case: https://gist.github.com/oclbdk/06312868b8836c16f982

It might be possible to package those types of dependencies as npm modules instead, as long as atom.styles.addStyleSheet() and atom.keymaps.loadKeymap() are recommended ways to include styles and keymaps in the npm modules.

oclbdk commented 9 years ago

I was discussing with @bolinfest and he brought up a good point against using npm modules -- they may be duplicated among many dependent Atom packages, which could cause Atom to reload many of the same stylesheets/keymaps and otherwise swallow up more disk space for the user.

maxbrunsfeld commented 9 years ago

the tests become a maintenance nightmare -- since tests don't activate any packages by default, we need to manually write atom.packages.activatePackage() for each recursive package dependency so their services are available.

This is a really good point. I'm thinking we could simply have a boolean flag called activateProviders on each consumedServices entry that indicates whether dependent packages providing the service should automatically be activated when the this package is activated:

{
  "consumedServices": {
    "my-widget": {
      "activateProviders": true,
      "versions": {
        "0.6": "consumeMyWidget"
      }
    }
  }
}
nathansobo commented 9 years ago

@oclbdk Thanks for your thoughts on this the and thoughtful proposal. My thoughts still aren't fully formed on this and I agree that it's an important problem.

I was thinking that our existing providedServices and consumedServices fields could be used for this, but that you could add a required field to any provided or consumed service, indicating a hard dependency. So required on a consumed service would mean "this package won't work if it can't consume this service". And a required on a provided service will mean "this package is useless if nothing consumes this service".

This seems like it is similar to your proposal without adding a lot of new apparatus to package.json, but maybe I'm not fully understanding your thinking. Another issue is for packages that purely exist to provide services, what does the UI in the settings view look like? Do we want to treat them specially somehow since they don't have a UI of their own? My concern is gunking up the UI with a ton of service provider packages that are really not of interest to the user.