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

allow plugins to depend on one another (breaking change) #141

Merged
merged 4 commits into from
Nov 20, 2015

Conversation

andyroyle
Copy link
Collaborator

  • deps must be static and exposed as an array of plugin names, e.g. module.exports.dependencies = ['myplugin', 'myotherplugin']
  • will detect circular dependencies and missing plugins
  • registers each plugin only when all it's dependencies have been registered
  • recursively registers in cases of deep dependency chains
  • passes dependencies as an object to the register method (e.g. use deps.myplugin() to execute the dep)

Adds an extra module dep madge, which seems pretty heavy (for the tiny piece we're using (checking for circular dependencies).

Not sure if it's worth finding an alternative or just pulling the logic out and wrapping it in a new package.

See what you think @matteofigus.

@matteofigus
Copy link
Member

👍 Thanks good job! I would leave the madge optimisation in the future in case.
Just give me some time to look at the PR - it's a lot of code.

PS
madge depends on coffeescript because of Gruntfile.coffee
ok

@andyroyle
Copy link
Collaborator Author

actually, fuck madge, I'm going to swap it out for this: https://www.npmjs.com/package/dependency-graph

@matteofigus
Copy link
Member

dependencies": {} 🍻

@andyroyle
Copy link
Collaborator Author

closes #128

@andyroyle
Copy link
Collaborator Author

@matteofigus you had a chance to look at this one?

matteofigus added a commit that referenced this pull request Nov 20, 2015
allow plugins to depend on one another (breaking change)
@matteofigus matteofigus merged commit a8ebaaa into master Nov 20, 2015
@matteofigus matteofigus deleted the plugin-dependencies branch November 20, 2015 13:40
@matteofigus
Copy link
Member

Published 0.20.0

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.

2 participants