Skip to content

Literals#27

Open
cadement wants to merge 2 commits intojaredhanson:masterfrom
Sharecare:literals
Open

Literals#27
cadement wants to merge 2 commits intojaredhanson:masterfrom
Sharecare:literals

Conversation

@cadement
Copy link

Hi there. I ran into issues converting from 0.0.6 to 0.1.0 when dealing with literal components. In 0.6.0, I was able to use IoC.singleton(key, value) to inject dependencies, but with 0.1.0 this functionality went away. I noticed you had an existing test that was broken for doing singleton injection, so I thought I'd take a swing at it.

Basically I've followed your "loaders" convention to create a new IoC.literal(key, value) loader usable with the use() method. So similar to the IoC.use(IoC.node(somePath)) syntax, you would use IoC.use(IoC.literal(id, component)) to inject a literal component.

Not sure if that's the way you had in mind for solving this issue - and if there's a different approach you'd prefer let me know and I'll work that up and submit it instead. Not a big coding effort - so reworks are cheap ; )

@jaredhanson
Copy link
Owner

The idea is you can do this the following way:

IoC.use(IoC.fs('app');

In app/myliteral.js:

exports = module.exports = 'bar';
exports['@literal'] = true;

To create the literal:

IoC.create('myliteral');

To inject the literal:


exports = module.exports = function(myliteral) {
...
}
exports['@require'] = [ 'myliteral' ]

@jaredhanson
Copy link
Owner

This is all a bit in flux at the moment, so let me know if you had some other idea.

@cadement
Copy link
Author

Hmmm. Let me show you a bit of the use-case I am wrestling with for context?

What I've got is an asynchronous config loader that returns a configuration object to the app, and then several of the core dependencies are built from that configuration. Here's a (very simplified) 0.0.6-style example:

configLoader
  .load(path)
  .then(function(config) {
    var IoC = require('electrolyte');

    IoC.singleton('log4js', require('log4js-config')(config.logging));

    IoC.singleton('mongoose', require('mongoose-config')(config.db.host, config.db.port, config.db.database));

    IoC.loader(IoC.node('lib'));

    var app = IoC.create('app');
    var server = http.createServer(app);
    ...
  })

So in a circumstance like this, I don't really know any way to build a source that synchronously returns the literal object, because there's no way to synchronously require the configuration into the source. That kinda boxed me in to having to manually create the objects directly, and then inject them into the container explicitly (which is where the old singleton() syntax came in very handy).

Honestly, it kinda reminds me of many a discussion I've had in Spring about when it is appropriate to annotate a class as a @Component vs when I need to create a @Configuration factory to build @Bean methods (or define beans in XML if you're old school). The approach of loading sources that are "annotated" is very analogous to the prior. But I think there may be value in the latter for some problems as well.

Or... I could just be missing something and overcomplicating the heck of this. It wouldn't be the first time...

@jaredhanson
Copy link
Owner

Cool. So, this is still somewhat experimental, but here's how I'm intending to handle this. See this file what I'm doing in my applications: https://github.com/bixbyjs/bixby-sd-etcd/blob/master/lib/registry.js

Another example:

exports = module.exports = function(settings) {
  var mysql = require('mysql');
  var options = settings.toObject();

  return mysql.createConnection({
    host     :  options.host,
    database : options.database
  });
}

exports['@require'] = [ '$settings' ];

The idea is that there is some settings external to the application (in a file, or whatever), which become a object which can be injected. Other components can then use those settings to return configured instances to the application.

Does something like that look reasonable for your use case?

@cadement
Copy link
Author

So... I've chewed on this for a day trying to figure out what I'm missing, but I'm just not seeing it. Feeling dense!

The model you describe doesn't seem to address the challenge - how do I make a settings object available in the container so that it can be injected into modules that need it if the settings are built asynchronously? In the case of bixby-sd-etcd/registry.js, for example, it's pretty easy to make something like that work if I can make a settings module that will return the settings object. But in a case like I described above, I cannot do that. I can make a module that returns a function that will take a callback that gets invoked later to return the settings - but since everything in the container evaluates synchronously that's not very helpful.

The only way I can think to do this without manually injecting a literal bean would be something much more complicated like this:

settings.js
(function() {
    var settings = {};
    var exports = module.exports = function() {
        this.init = function(newSettings) {
            settings = newSettings;
        };
        this.getSettings = function() {
            return settings;
        };
    }
    exports['@singleton'] = true;
}).call(this);
www.js
configLoader
  .load(path)
  .then(function(config) {
    var settingsRepo = require('./lib/settings')();
    settingsRepo.init(config);

    var IoC = require('electrolyte');

    IoC.loader(IoC.node('lib'));

    var app = IoC.create('app');
    var server = http.createServer(app);
    ...
  })

And then I could @require settings into my modules and use the getSettings() method to access the settings since I've . But that just seems like such a counter-intuitive and non-expressive approach compared to the very direct and understandable act of saying "add this literal to the container with this id".

Am I just misunderstanding?

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