Registering the same type twice in a single module should provide an error rather than silent override of the type.

[mhevery]

This should be an error.

new Module()
  ..value(String, null)
  ..value(String, '');

There is no reason why within a single module one should be able to give two different definitions to the same type, and is most likely an error and should be reported.

[mvuksano]

How about situation where there are two modules? let's call them m1 and m2. m2 is installed in m1 like:

class m1 extends Module {
  m1() {
    value(String, 'abc');
    install(m2);
  }
}

and we have m2 defined as

class m2 extends Module {
  m2() {
    value(String, 'def');
  }
}

Should this be error too? Or should there just be a warning for this situation? I'm leaning towards warning...

[vicb]

Ont solution would be to add the allowOverridehere also. @markovuksanovic it would be great if you check in Guice also. Thanks.

[pavelgj]

There are a couple of use-cases here:

1. shooting yourself in the foot -- probably the one @mhevery is talking about
2. binding overridability controls

While I agree with the first case, I'd argue to hold off on the second one until we understand the problem a little better.

Module designer might not intend for certain types to be overridden by the user, on the other hand everything should be overridable for testing. So the question is, should we force the user to provide mocks via separate module?

Disallow this?

var module = new AngularModule()
  ..type(Parser, implementedBy: MockParser);
 var injector = new DynamicInjector([module]);

Only allow this?

var mockModule = new MockModule()
  ..type(Parser, implementedBy: MockParser);
var injector = new DynamicInjector([new AngularModule(), mockModule]);

[mvuksano]

I agree with @pavelgj . I think we should hold off the second use case. Regarding that, I think when one wants to override some binding it should be very explicit. I'd suggest something like

module.override(Parser).with(MockParser)

or something similar.

I'd suggest for now, if there's any kind of double binding to throw an exception. Thoughts?

[0x3333]

What about test cases? Which is the default implementation?

My application has a default Module which has the production bindings. In the test cases I provide a Module with some bindings to the bootstrap code which will override some default bindings, actually some mock views.

Currently I'm installing a optional module parameter after all my bindings, overriding default bindings, but I don't know if this is the "right thing".

I'm not using Angular-Dart, just the DI package.