danfang / me-api

An extensible, personal API with custom integrations
http://api.danielfang.org
MIT License
827 stars 42 forks source link

How external modules/dependencies would work #2

Open danfang opened 9 years ago

danfang commented 9 years ago

Right now, the app looks for specific modules outlined in modules.json in the lib/middleware/ folder. It also pulls all dependency requirements per module from the global package.json.

I was wondering:

  1. If people were to build other npm packages as extensions/alternative modules, how would I allow users to install those? I guess the more specific question is, do I look for global modules as well? E.g. instead of this.use(path, require("./middleware/" + module)), maybe something like this.use(path, require(module) || require("./middleware/" + module)).
  2. Design-wise is it better to remove all the integration dependencies (client libraries) from package.json? That way, the user won't have to install packages for integrations they might not use. Or maybe better yet, there is some way to choose the integrations while they are doing npm install. Food for thought.
therebelrobot commented 9 years ago

I like both of these ideas. You would, though, need to figure out a standard naming convention for the middleware, something like my-api-twitter or my-middleware-twitter or something, that way people know what to name their own custom middleware on npm, and they know what to look for when finding ones that may already exist. I'd be happy to help out with the splitting out of the current middleware if you need assistance :)

sean-clayton commented 9 years ago

How about a structure like this?

var myApi = require('my-api');
var myMiddleware = require('myMiddleware');

myApi.use('myMiddleware');

This way, you won't need a naming convention that @therebelrobot mentioned and you still get modules :)

therebelrobot commented 9 years ago

That would definitely work, though you'd want to call the middleware by the variable name you declared, rather than the string, so like this:

var myApi = require('my-api');
var myMiddleware = require('myMiddleware');

myApi.use(myMiddleware);

The naming convention would be more for npm package searching convenience, like grunt-plugin or gulp-plugin, etc.

therebelrobot commented 9 years ago

Though that would also require hard-coding the require into place, and I think he wants a more programmatic require in place, so it can be passed in as an option on the command line or in the build script.

danfang commented 9 years ago

Proposed changes:

{
  "modules": [
    "customMiddleware": {
       "custom": true, "path": "/", "data": {} 
    }
  ]
}
if (module.custom) require(module)
else require("./middleware/" + module);

How does that sound?
marvinroger commented 9 years ago

I think this is a good thing, for now, to avoid the need to write code to use Me API, so registering custom modules in modules.json is the finest to me. However:

if (module.custom) require(module);
else require(__dirname + "/middleware/" + module);

This only allows the use of "native" modules (bundled), and NPM modules. However, what if I want to use a very custom middleware, that I don't want to share on NPM? Maybe we could do something like:

{
  "modules": [
    "customMiddleware": {
       "type": "custom", "path": "/", "data": {},
       "type": "module", "path": "/", "data": {},
       "type": "bundled", "path": "/", "data": {} // Maybe omit `type` for bundled
    }
  ]
}
if (module.type == "bundled" || typeof module.type == "undefined") require(__dirname + "/middleware/" + module);
else if (module.type == "module") require(module);
else if (module.type == "custom") require("./middleware/" + module);

This is more flexible I think.

Regarding the naming convention @therebelrobot suggested, this is a must and would better be something like Slush (e.g. me-api-* as a name and me-api-middleware as a keyword, so that a search to NPM would return every results easily).

marvinroger commented 9 years ago

About the second point, letting the user choose the integrations during the npm install is I think, not a good idea. A npm install should not be blocking. As an example, I am using Me API through a PaaS, so I have no control over the NPM install phase. So instead of npm install, this should be done in me-api-init. Do you guys agree?

therebelrobot commented 9 years ago

@marvinroger +1 on the init, and the Slush comparison, that's what I was getting at for naming conventions.