mariocasciaro / scatter

IoC container and out-of-the-box extensibility for Node.js applications
MIT License
154 stars 14 forks source link

Support dependencies with relative paths #13

Closed k7sleeper closed 10 years ago

k7sleeper commented 10 years ago

Dependencies starting with ./ or ../ should be resolved against the directory of the dependent module.

For me this feature would be very useful but I cannot figure out if this feature is in conflict with other features of Scatter.

mariocasciaro commented 10 years ago

This is also a nice to have feature which I can't see though all the impact it can have and we should also consider if the effort is reasonable compared to its advantages. One side effect is that relative namespaces would not work if the module is loaded from the Scatter object (because it does not belong to any namespace).

However it should not be impossible to implement, but I already see it would require a change in the Resolver API, which now would need to resolve a module from a relative namespace. We could create a branch and create some tests, just to see how it "feels" to use relative dependencies.

On Thu, Dec 12, 2013 at 8:08 AM, Klaus Sevensleeper < notifications@github.com> wrote:

Dependencies starting with ./ or ../ should be resolved against the directory of the dependent module.

For me this feature would be very useful but I cannot figure out if this feature is in conflict with other features of Scatter.

— Reply to this email directly or view it on GitHubhttps://github.com/mariocasciaro/scatter/issues/13 .

k7sleeper commented 10 years ago

Well, what's your suggestion for the following task?

I've the following folder structure:

/app
  /particle
    /comp1
      a.js
      b.js  
    /comp2
      c.js
    /core
      main.js

I register app/particle at Scatter.

Module core/main.js depends on comp2/cand comp1/b. comp1/b depends on comp1/a. Thus, in app/particle/comp1/b.js there's code like

module.exports.__module = {
  args: ['comp1/a']
};

Wouldn't it be preferable to have code like that in app/particle/comp1/b.s?

module.exports.__module = {
  args: ['./a']
};

Is it only nice to have, or must-have ? Do I oversee anything or do I have the wrong approach ? Would namespaces be the right solution of that task?

k7sleeper commented 10 years ago

What do you mean with "if the module is loaded from the Scatter object" ?

mariocasciaro commented 10 years ago

This is a nice to have feature, it is definitely not necessary, and from what I can see it could let the user think there is some sort of similarity between Scatter dependencies and require, which might be confusing. On the other side it could make dependencies less verbose in some circumstances.

For the other question, this will never work with the current implementation:

var scatter = new Scatter();
[...]
scatter.load("./a").then(...)

because it does not live in a Scatter module (of course it is the container itself), so it does not have a namespace, and therefore relative namespaces cannot be calculated.

k7sleeper commented 10 years ago

My thoughts are, if I rename /comp1 in my example above or use comp1 in another project in which I have to save it under an other name or in a different folder level (deeper or higher), then I also have to change the files under comp1 (or rather the new name). That's unfavorably, isn't it?

Syntax for namespace relative deps could be '#/a' instead of ./a to distinguish Scatter dependencies from require() and Scatter namespaces from directories.

var scatter = new Scatter();
[...]
scatter.load("#/a").then(...)

should not be allowed.

mariocasciaro commented 10 years ago

Yes, as I said it has some advantages and we can definitely try to get a prototype in a separate branch (not the master) since it can have an impact on a few things, and see how it works. Then we can merge in master as soon as we are happy with the result.

For the syntax the #/ is not really intuitive for the user, so if we are going ahead with this the ./ is definitely the way to go. We will then update the docs to reflect the new feature and we make sure it is clear to the user that relative namespaces are different than relative directories.

k7sleeper commented 10 years ago

Sounds good.

k7sleeper commented 10 years ago

Works very fine, and is essential for me :100: !

Thanks!

mariocasciaro commented 10 years ago

Happy it helped :)