Add option to allow prioritizing of sources

[dpatti]

When you invoke container.load(), it pushes the new source onto the end of an array of sources. What I am trying to do is the opposite -- I want to take my application's existing container, and when setting up the test suites, add a source whose components will shadow the existing ones. Right now, I can do this with _registerModule(), but that is obviously private API and not to mention quite clunky.

Is there a reason that you push to sources instead of unshifting? I would normally expect those added later to take precedence, but I'm not sure if there are situations I haven't yet considered. Perhaps there is also a better solution that I haven't considered yet.

[jaredhanson]

Yeah, I like pushing, so sources reads in "priority" order, top-to-bottom. For example, I have an app that looks like:

IoC.loader('handlers', IoC.node(__dirname + '/handlers'));
IoC.loader(IoC.node(__dirname + '/components'));
IoC.loader('statsd', require('bixby-statsd'));
IoC.loader('pki/self', require('bixby-pki'));
IoC.loader('mq/tasks', require('bixby-crane'));
IoC.loader('sd', require('bixby-sd'));
IoC.loader(require('bixby-common'));
IoC.loader(require('bixby-express'));

So, I have a bunch of common stuff at lowest priority, and could override anything by creating a component in the app's components directory.

Your case is valid though. I'd be in favor of passing an option that does what you describe. Something like:

IoC.loader(IoC.node(__dirname + '/components'), { unshift: true });

[dpatti]

Neat, thanks. I'll take a stab at it this week.

[dpatti]

Okay, I've added a commit with some initial work on this. I thought that maybe unshift or prepend might be bad option names since it would imply the user has knowledge of how containers store their sources, but maybe they're still more clear than prioritize, which I went with. Your call on that one.

Another possibility that I thought of that you might find more appealing is to separate the adding of the source into a separate private method so that a user can extend the Container class to implement their own options by overriding the method. I would be happy to experiment with this route if you think it has merit.

[dkushner]

@dpatti, @jaredhanson. I vote against relying on object extension/method overriding for the end user. Why not simply a third, boolean parameter in the IoC.loader() function that dictates whether we are supplanting the current source for a module? It would only ever need to be considered for the 2-parameter signature of the current function.

Side question, @jaredhanson, is there a reason it needs to maintain an array of sources defined for a particular module? Would it not make more sense to have a single map (component name) --> (source) and have subsequent registrations with that name displace the existing source?

[jaredhanson]

Sources provide one or many components. The array is similar to a search path for executables. The component -> module map is a map (the _o ivar).

[dkushner]

@jaredhanson, ah that makes much more sense. Do you have a quick example of how a source can provide multiple components? I've been using the single-file component structure demonstrated in the Express sample app and its been a little bit cumbersome.

Or does a 'source' refer generally to both a directory containing multiple component definitions or a single source file containing a component definition?

[jaredhanson]

A source refers to anything that supplies components. In many cases, this is simply a directory, as indicated by:

    IoC.use(IoC.node('app/components'));

The directory works best for application-specific components that aren't generally reused in other apps.

It may also be a specifically designed "component suite" as I refer to them. You can see these with any of the Bixby.js suites I'm currently working on, registered as:

    IoC.use(IoC.node('app/components'));
    IoC.use('pki', require('bixby-pki'));
    IoC.use('sd', require('bixby-sd'));
    IoC.use(require('bixby-common'));

For example, bixby-pki provides the components listed in its index.js.

Note that in this case, any source above the others takes priority. So, if you didn't like the default logger in bixby-common you could overwrite it by implementing a logger.js inside app/components or by useing your own suite of components above bixby-common.

In this way you can pull out a lot of the boilerplate infrastructure that an app needs into easily sharable modules.

[dpatti]

@jaredhanson Do you have any feelings on the code I've added?

[dkushner]

@jaredhanson, ah okay. I can definitely understand the motivation, though to have priority work from the top down is the bit that confuses me. I mean this runs counter to the procedural nature of JavaScript where subsequent assignments to some variable overwrite previous ones? Is there a way we can specify that newly loaded components should always be unshifted when they overlap with an existing definition? This would fix the logical disconnect and improve the usability for testing purposes that @dpatti seems to be going for (and I would also appreciate).

[jaredhanson]

As a counterpoint, if you app.use middleware in an express app, they will be run top-down in the order listed

I'm not sold that one is naturally better than the other, and any confusion is easily resolved by reading documentation

Sent from my iPhone

On Dec 17, 2014, at 10:40 AM, Dave Kushner notifications@github.com wrote:

@jaredhanson, ah okay. I can definitely understand the motivation, though to have priority work from the top down is the bit that confuses me. I mean this runs counter to the procedural nature of JavaScript where subsequent assignments to some variable overwrite previous ones? Is there a way we can specify that newly loaded components should always be unshifted when they overlap with an existing definition? This would fix the logical disconnect and improve the usability for testing purposes that @dpatti seems to be going for (and I would also appreciate).

— Reply to this email directly or view it on GitHub.

[jaredhanson]

Further thinking: I also prefer the top-down ordering of component sources as it most naturally resembles a "stack" that you are assembling:

IoC.use(IoC.node('app/components'));
IoC.use('pki', require('bixby-pki'));
IoC.use('sd', require('bixby-sd'));
IoC.use(require('bixby-common'));

The app builds on top of PKI builds and service discovery, which build on top of common components, etc.

[dpatti]

In that same sense, another possible solution instead of prepending a loader to the beginning of the chain would be to somehow use an existing container as a loader too. Something like:

appContainer = require('./app/container')
testContainer = new Container()
testContainer.use(IoC.node('./test/components'))
testContainer.use(appContainer)

I know this probably doesn't mesh with the internals well as-is, but the concept of making containers more composable would solve a lot of problems.

[dkushner]

@jaredhanson, I see what you're getting at in regards to the visual structure of the statements matching a concept, but to me this does not seem nearly enough reason to violate the procedural nature of the language itself, let alone the fact that it makes testing using electrolyte (one of the primary advantages of IoC) really non-obvious. I like @dpatti's solutions, they're good compromises but they're half measures to cover what I feel is a poor design decision. It is, after all, your project and an otherwise excellent one at that) so feel free to disregard this simple criticism. However, this is currently keeping me from rolling electrolyte into our application.

[jaredhanson]

I've changed my mind on this topic. Component sources are now unshifted onto the array as they added. This is in master now, and will be in the next release of Electrolyte. Thanks for the discussion!