angular
angular copied to clipboard
Should we infer @Optional based on argument type?
Had a nice discussion today, why do we even have @Optional
?
class Animal {
Animal(@Optional() Ecosystem ecosystem);
}
... since Dart has this already, in both positional and named arguments:
class Animal {
Animal([Ecosystem ecosystem]);
}
class Animal {
Animal({Ecosystem ecosystem});
}
It seems safe to say we could infer all optional parameters as @Optional()
, and remove this key word entirely. Two potential wrinkles though that need further discussion:
- How do we treat named arguments annotated with
@required
? - How do we treat named or positional optional arguments with default values?
i.e.:
class Animal {
Animal({@required Ecosystem ecosystem});
}
My intuition here is to treat this as required (not optional).
class Animal {
Animal({Ecosystem ecosystem: const DefaultEcosystem()};
}
My intuition here is to either:
a. Ignore and do not support for DI
b. Try to use the same "reviving" logic we use for useValue
:
new Animal(ecosystem: injector.getOptional(Ecosystem) ?? const DefaultEcosystem())
A final question is whether we need something like @ignore
. My intuition is no.
I think the optional named/unnamed parameters change is likely a much bigger change, and not in scope for 5.x unless we are very motivated (it would require re-ordering parameters in client code). I think that rolling it our incrementally as an optional feature for ^5.1.0 is more likely. Ideas:
class A {
@Inject.useOptionalParameters()
A(B b, [C c]);
}
... other names, also welcome :)
I'd be okay with simply ignoring @required
or perhaps warning users it won't be honored.
I see the annotation's purpose as a way of using named parameters to make your functions self-documenting at their call-site. But with Angular, users are almost never exposed to the call-sites of component constructors, so it loses its purpose.
I'm in favor of using the same reviving logic to use the default values when injection fails.
We could even choose to only support unnamed optional parameters to avoid the issue of @required
, based on the same argument that named parameters are self-documenting at call-sites, which again don't matter in our case.
@leonsenft Luckily we don't support named arguments at all (https://github.com/dart-lang/angular/issues/448), so we can skip that for now.
Accepted, but we'll do this >=5.1 <6.0.