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

Extending DI into loaded modules #28

Open
cadement opened this issue Oct 26, 2015 · 1 comment
Open

Extending DI into loaded modules #28

cadement opened this issue Oct 26, 2015 · 1 comment

Comments

@cadement
Copy link

More of a topic for discussion than an issue. I've been thinking quite a bit lately about how DI in general, and electrolyte specifically, might be able to support injection in loaded dependencies (i.e. other projects I've specified as dependencies in package.json). I think I've got the basic framework of a solution in place, so with straw man in hand it's time for outside opinions!

First, let me state conceptually what I think DI in dependencies means. Basically I was looking for a few things:

  • Dependencies should be available as modules to the parent app container (what IoC.node_modules() currently accomplishes - this should not break!).
  • Modules declared in the parent container should be injectable into modules inside of dependencies. So a logger configured in the main app should be usable even inside of modules in a loaded dependency).
  • Modules declared in the dependency should NOT be injectable into modules in the main application. The API of a dependency should be inviolate and implementation details should not leak.
  • Within a dependency, modules references should be addressable directly without any sort of special syntax to identify the dependent project (e.g. exports['@require'] = ['userRepository'];, not exports['@require'] = ['myProject/userRepository'];).

Okay, so with that goal in mind and after quite a bit of trying to understand the guts of electrolyte, what I'd like to propose is adding to Container the concept of an optional "parent". Each library which wants to support DI would then have the container of the parent app injected into their main file, and use that container as the parent of a container local to that library. When a container is asked to create a module, it then attempts to create that module locally and, if unsuccessful and has a linked parent container, defers to the parent to attempt to create the module. And that's pretty much it.

So what does this look like in practice? Essentially, the main file of a library looks very much like the main file of the parent app, AND it looks a bit like a module spec - because now it's actually both. For example:

var electrolyte = require('electrolyte'),
    path = require('path');

var exports = module.exports = function(parentContainer) {
  var ioc = require('electrolyte')(parentContainer);
  ioc.use(electrolyte.node(path.join(__dirname, 'lib')));
  return ioc.create('myService');
};
exports['@singleton'] = true;
exports['@require'] = ['!container'];

And then any modules inside of the lib directory of this library are in turn configured by the container as needed using either other modules inside this library or any needed modules from the parent container. And as you might suspect, this hierarchical pattern is completely repeatable - there's no reason this library could not in turn load dependencies of its own that are similarly configured (and on and on as many levels deep as required).

What's it take to make that work? Actually very little:

  • Add _parent to Container
  • Add logic to Container.prototype.create to fail up to the parent if spec is not resolved locally
  • Modify the main index.js to allow for optionally passing in a parent container instead of always constructing a raw new Container()

And that's it? Well, no, not it. Remember the first bullet in the objectives - making this work with the existing .node_modules(). That I have not done yet and it's going to take a little creativity...

Also, I'm not very happy with the fact that the libraries have to explicitly prefix their local paths with __dirname for all of their use() statements. Haven't figured out how to work around that yet :'(

But other than that, the above solution seems to actually work very well.

Okay - straw man erected and sticks passed around to the group. Begin with the beating :)

@cadement
Copy link
Author

Nope, I was wrong. Turns out it was really easy to patch up IoC.node_modules() to do this. Looks like:

module.exports = function(options) {

  return function(id) {
    try {
      var module = require(id);
      if (!module['@require']) {
        module['@literal'] = true;
      }
      return module;
    } catch(E) {
      return;
    }
  }
}

So.. nobody has taken up the opportunity to tell me what a bad idea this is... Does that mean I should start thinking about submitting a PR? O:)

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

No branches or pull requests

1 participant