Clean up / document client side loading.

[subhog]

You've got many classes for module loading! ModuleManager, ModuleConstructor, _ModuleManagerConstructor, ModuleInstance...

• I know you had a reason, but cannot it be a bit simpler?
• If it cannot, provide a basic documentation. I think that providing readme.md in every directory is a good pattern and we may start following it.

The reason why I'm pushing this that I need to do some things with instances due to module development, and I cannot do these easily.

• I have module instance and I need its prefix. How do I get it?
• Module instance has render method even if I didn't specify it. Why? I consider it a bug (it's certainly asking for one). There should be an object which holds everything that developer exports and nothing else. Otherwise we need to provide restricted words list, and this isn't the direction we're looking for.

  Dangerous:

I've hard-coded prefix creation in yield/helpers.js. Another prefix creation is on the server. Keep this note open until resolved.

[subhog]

Also important: the state object sent to module.render / module.routes / Router should only implement necessary methods (getPath, getParams, matchRoute). It shouldn't allow to tamper with _listeners for instance.

[subhog]

I don't like the pattern with factory creation in module: new Impact.ModuleFactory();. Usually the new keyword is used to create a new object that has to be taken care of, here it's just hanging around. Placing the object in memory shouldn't happen in constructor.

It should be easy to switch the constructor and registerFactory methods so that the latter one is called from the module, and invokes constructor inside.

[subhog]

I don't like that I've placed matchRoute in yield.js either.

[apendua]

@subhog You say:

Module instance has render method even if I didn't specify it. Why? I consider it a bug (it's certainly asking for one)

That's right. The module instance has a prototype that defines default render function, which only displays an error message. Since the existance of proper render method is quite important I guess it is a safe solution. Right?

[subhog]

The module instance has a prototype that defines default render function, which only displays an error message. Since the existance of proper render method is quite important I guess it is a safe solution. Right?

Wrong. Again, there should be an object which holds everything that developer exports and nothing else. This is important for simplicity, as otherwise we should instruct module developer on what's also in this object (and then we'll not be free to adjust it). Existence of render function is not that important as there may be other ways to communicate with the module in the future (one sketch of such possibility is already in place).

[apendua]

@subhog Heh... it's not a property of the exports object. What module developer has added to its exports gets merged into our module instance. The latter has prototype with default implementation of render and also a single private _impact key (for our use) that is ignored on merge - so the developer cannot overwrite this even if he want's to (he gets a warrning message if he tries to). So we do not have to inform the developer about anything (maybe just 'avoid _impact key'). Supposing that we will need more than just render, the use of object's prototype is alway a nice and clean way to provide default implementation. After all this is what prototyping is designed for.

[subhog]

@subhog Heh... it's not a property of the exports object. What module developer has added to its exports gets merged into our module instance.

I know. And that's what I don't like.

See how node modules are done. Instance of the module is exactly what was added to exports, and nothing more. That's how it should be. I can think of no reason why to defy this simplicity.