From 961210a79b1424b7f89fecfe667c825a224ce997 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Thu, 1 Mar 2018 22:11:01 -0800 Subject: [PATCH 1/5] doc(Core): Start a guide on best DI practices. --- doc/guide/di.md | 146 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 146 insertions(+) create mode 100644 doc/guide/di.md diff --git a/doc/guide/di.md b/doc/guide/di.md new file mode 100644 index 0000000000..92b5aa5fd1 --- /dev/null +++ b/doc/guide/di.md @@ -0,0 +1,146 @@ +# Dependency Injection + +There are many different ways to use AngularDart to provide and utilize dependency injection throughout your application, components, and services. This guide is meant to serve as a **development tool** and to help push **best practices** from the developers of the AngularDart framework. + +* [Providers](#providers) + * [DO use the "named" providers](#do-use-the-named-providers) + * [DO use factories for configuration](#do-use-factories-for-configuration) + * [DO use `const` providers](#do-use-const-providers) +* [Injectors](#injectors) + * [AVOID using `ReflectiveInjector`](#avoid-using-reflectiveinjector) + +## Providers + +### DO use the "named" providers + +While not formally deprecated, `provide(...)` and `Provider(...)` require certain combinations of named optional parameters, and allow combinations that otherwise will not compile or perform as intended. For example: + +**BAD**: + +```dart +// This is valid with the older syntax. What does it mean? +const Provider( + Foo, + useClass: CachedFoo, + useExisting: OtherFoo, + useFactory: createCachedFoo, + useValue: null, +); +``` + +Instead, use the "named" providers (`ClassProvider`, `ExistingProvider`, `FactoryProvider`, and `ValueProvider`) in order to make your providers more readable, in some cases more terse, and use the definition of the class to help guide the configuration. + +**GOOD**: + +```dart +const ClassProvider(Foo, CachedFoo); +const ExistingProvider(Foo, OtherFoo); +const FactoryProvider(Foo, createCachedFoo); +const ValueProvider(Foo, null); +``` + +### DO use factories for configuration + +As part of [avoiding runtime configured providers](#do-use-const-providers), you may have or desire patterns where dependency injection varies based on some runtime information, or information that is otherwise not expressable as a constant. Use `FactoryProvider` instead. + +**BAD**: + +```dart +Provider bindUser(Flags flags) { + if (flags.isAdminUser) { + return new ClassProvider(User, useClass: AdminUser); + } + return new ClassProvider(Use, useClass: RegularUser); +} +``` + +**GOOD**: + +```dart +const FactoryProvider(User, useFactory: createUserFromFlags); + +User createUserFromFlags(Flags flags) { + return flags.isAdminUser ? new AdminUser() : new RegularUser(); +} +``` + +### DO Use `const` providers + +It can be tempting to create and configure providers at runtime, either using `new Provider(...)` or `provide(...)`. While not formally deprecated, runtime configuration of providers relies on side-effects in your program, and [should be avoided](#avoid-using-reflectiveinjector). + +**BAD**: + +```dart +void main() { + var injector = ReflectiveInjector.resolveAndCreate([ + new ClassProvider(HeroService), + ]); +} +``` + +Even if you still rely on this API partially, new providers are highly recommended to use `const` in order to be able to better injectors in the future: + +**GOOD**: + +```dart +void main() { + var injector = ReflectiveInjector.resolveAndCreate([ + const ClassProvider(HeroService), + ]); +} +``` + +## Injectors + +### AVOID using `ReflectiveInjector` + +While not formally deprecated, `ReflectiveInjector` relies on importing `.dart` files having side-effects on your program, and negatively effecting tree-shaking by making all classes and functions annotated with `@Injectable` retained, regardless of use. + +**BAD**: + +```dart +void main() { + var injector = ReflectiveInjector.resolveAndCreate([ + const ClassProvider(HeroService), + ]); +} +``` + +AngularDart gives a few alternative options when creating injectors. + +**GOOD**: Use `Injector.map` to create a simple dynamic injector. + +```dart +void main() { + var injector = new Injector.map({ + HeroService: new HeroService(), + }); + // ... +} +``` + +**GOOD**: Use `providers: const [ ... ]` in `@Component` or `@Directive`. + +```dart +@Component( + selector: 'comp', + providers: const [ + const ClassProvider(HeroService), + ], +) +class Comp {} +``` + +**GOOD**: Use `@GenerateInjector` to generate an injector at compile-time. + +```dart +// assume you are currently in file.dart +import 'file.template.dart' as ng; + +@GenerateInjector(const [ + const ClassProvider(HeroService), +]) +final InjectorFactory heroInjector = ng.heroInjector$Injector; +``` + +## Components \ No newline at end of file From 3798bfa656312670e571f9d222db93e657ab9f36 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Fri, 2 Mar 2018 13:00:22 -0800 Subject: [PATCH 2/5] More docs. --- doc/guide/di.md | 160 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 154 insertions(+), 6 deletions(-) diff --git a/doc/guide/di.md b/doc/guide/di.md index 92b5aa5fd1..6d03b7bc7d 100644 --- a/doc/guide/di.md +++ b/doc/guide/di.md @@ -1,13 +1,20 @@ -# Dependency Injection +# Effective Angular: Dependency Injection + +> **NOTE**: This is a work-in-progress, and not yet final. Some of the links may be substituted with `(...)`, and some TODOs or missing parts may be in the documentation. There are many different ways to use AngularDart to provide and utilize dependency injection throughout your application, components, and services. This guide is meant to serve as a **development tool** and to help push **best practices** from the developers of the AngularDart framework. * [Providers](#providers) - * [DO use the "named" providers](#do-use-the-named-providers) - * [DO use factories for configuration](#do-use-factories-for-configuration) - * [DO use `const` providers](#do-use-const-providers) + * [DO use the "named" providers](#do-use-the-named-providers) + * [DO use factories for configuration](#do-use-factories-for-configuration) + * [DO use `const` providers](#do-use-const-providers) + * [DO use the `.forToken` constructor for tokens](#do-use-the-fortoken-constructor-for-tokens) +* [Tokens](#tokens) + * [DO use typed `OpaqueToken`](#do-use-typed-opaquetokent) + * [AVOID using arbitrary tokens](#avoid-using-arbitrary-tokens) + * [PREFER `MultiToken` to `multi: true`](#...) * [Injectors](#injectors) - * [AVOID using `ReflectiveInjector`](#avoid-using-reflectiveinjector) + * [AVOID using `ReflectiveInjector`](#avoid-using-reflectiveinjector) ## Providers @@ -33,7 +40,7 @@ Instead, use the "named" providers (`ClassProvider`, `ExistingProvider`, `Factor **GOOD**: ```dart -const ClassProvider(Foo, CachedFoo); +const ClassProvider(Foo, useClass: CachedFoo); const ExistingProvider(Foo, OtherFoo); const FactoryProvider(Foo, createCachedFoo); const ValueProvider(Foo, null); @@ -90,6 +97,147 @@ void main() { } ``` +### DO use the `.forToken` constructor for tokens + +In alignment with [DO use typed `OpaqueToken`](#...) and [DO use the "named" providers](#...), the `.forToken` constructor should be used in order for type inference to determine the type of the provider. + +**BAD**: + +```dart +const appBaseHref = const OpaqueToken('appBaseHref'); + +// This is created as `Provider`. +const Provider(appBaseHref, useValue: '1234'); +``` + +**GOOD**: + +```dart +const appBaseHref = const OpaqueToken('appBaseHref'); + +// This is created as `Provider`. +const ValueProvider.forToken(appBaseHref, '1234'); +``` + +## Tokens + +### DO use typed `OpaqueToken` + +In the past the `OpaqueToken` class was only useful as a convention for declaring arbitrary tokens (i.e. that were not based on a class `Type`). For example, you wouldn't want to bind something to `String` (too common), so you'd create an `OpaqueToken`: + +```dart +const appBaseHref = const OpaqueToken('appBaseHref'); +``` + +However, this also meant that it was undocumented (unless manually added with dartdocs) what the expected _type_ of `APP_BASE_HREF` was (in this case a `String`). This hurt both developer productivity _and_ the compiler (we don't know what to expect, so we typed it as `dynamic` always). + +**BAD**: Using `OpaqueToken` implicity or explicitly. + +```dart +// If not defined, const OpaqueToken(...) is of type . +const appBaseHref = const OpaqueToken('appBaseHref'); +``` + +**GOOD**: Using `OpaqueToken` where `T` is not `dynamic`: + +```dart +const appBaseHref = const OpaqueToken('appBaseHref'); +``` + +**GOOD**: Use your own `class` that extends `OpaqueToken`. + +Another option is to create your own "custom" token type. It's up to your and your team whether this is more ergonomic/documenting/clarifying than relying on a string-based description (`'appBaseHref'`). + +```dart +class AppBaseHref extends OpaqueToken { + const AppBaseHref(); +} + +// Optional. +const appBaseHref = const AppBaseHref(); + +// Can be used with .forToken now, for example: +const ValueProvider.forToken(appBaseHref, '/'); + +// Or as an parameter for injection: +class Comp { + Comp(@appBaseHref String href) { ... } +} +``` + +### AVOID using arbitrary tokens + +With the older style `Provider(...)` and `provide(...)` it's still type-safe and permitted to use arbitrary tokens, such as strings, numbers, or even custom classes. There are some bugs in the compiler, and it's unlikely support for this feature will be kept long-term in AngularDart. + +**BAD**: + +```dart +const Provider('Hello', useValue: 'Hello World'); +``` + +_or_ + +```dart +class HelloMessage { + const HelloMessage(); +} + +// In later configuration. +const Provider(const HelloMessage(), useValue: 'Hello World'); +``` + + +**GOOD**: Use a `Type` or `OpaqueToken` instead. + +```dart +const helloMessage = const OpaqueToken('Hello'); + +// In later configuration. +const ValueProvider.forToken(helloMessage, 'Hello World'); +``` + +_or_ + +```dart +class HelloMessage extends OpaqueToken { + const HelloMessage(); +} +const helloMessage = const HelloMessage(); + +// In later configuration. +const ValueProvider.forToken(helloMessage, 'Hello World'); +``` + +### PREFER `MultiToken` to `multi: true` + +Providers can be created with explicit configuration of `multi: true`, which means that when they are injected a `List` is returned instead of all tokens that are configured for that token/type. + +**BAD**: Using `multi: true` for this configuration. + +```dart +const usPresident = const OpaqueToken('usPresident'); + +const usPresidentProviders = const [ + const Provider(usPresident, useValue: 'George', multi: true), + const Provider(usPresident, useValue: 'Abe', multi: true), +]; +``` + +This pattern is _verbose_, _error prone_, and not very self-documenting for users. You could forget `multi: true`, clients would not know that injecting `usPresident` gives them a `List`. + +**GOOD**: Using `MultiToken` for this configuration instead. + +```dart +const usPresidents = const MultiToken('usPresidents'); + +const usPresidentsProviders = const [ + const ValueProvider.forToken(usPresidents, 'George'), + const ValueProvider.forToken(usPresidents, 'Abe'), +]; +``` + +As an added bonus, this pattern works better for Dart2; we're able to tell, statically, in the compiler, that `usPresidents` must return `List` not a `List`. + ## Injectors ### AVOID using `ReflectiveInjector` From 9ead30aedb654344540c71055d05e5739ee1f68a Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Fri, 2 Mar 2018 13:00:51 -0800 Subject: [PATCH 3/5] Move from /guide to /effective. --- doc/{guide => effective}/di.md | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename doc/{guide => effective}/di.md (100%) diff --git a/doc/guide/di.md b/doc/effective/di.md similarity index 100% rename from doc/guide/di.md rename to doc/effective/di.md From 7f7ee3a2eb061ab21212db0b5e48eaf38790799a Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Fri, 2 Mar 2018 13:04:36 -0800 Subject: [PATCH 4/5] Add a bit on implicit Type->ClassProvider. --- doc/effective/di.md | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/doc/effective/di.md b/doc/effective/di.md index 6d03b7bc7d..04917eaab2 100644 --- a/doc/effective/di.md +++ b/doc/effective/di.md @@ -9,6 +9,7 @@ There are many different ways to use AngularDart to provide and utilize dependen * [DO use factories for configuration](#do-use-factories-for-configuration) * [DO use `const` providers](#do-use-const-providers) * [DO use the `.forToken` constructor for tokens](#do-use-the-fortoken-constructor-for-tokens) + * [PREFER `ClassProvider` to a `Type`](#...) * [Tokens](#tokens) * [DO use typed `OpaqueToken`](#do-use-typed-opaquetokent) * [AVOID using arbitrary tokens](#avoid-using-arbitrary-tokens) @@ -119,6 +120,30 @@ const appBaseHref = const OpaqueToken('appBaseHref'); const ValueProvider.forToken(appBaseHref, '1234'); ``` +### PREFER `ClassProvider` to a `Type` + +While slightly more terse, the following pattern is not recommended: + +```dart +class HeroService {} + +const someProviders = const [ + HeroService, +]; +``` + +> In future versions of AngularDart, a new `Module` class will _require_ that all providers are explicitly of type (or super-type of) `Provider`, and `Type` will not be allowed: [https://github.com/dart-lang/angular/issues/543](https://github.com/dart-lang/angular/issues/543). + +**GOOD**: Use `ClassProvider` instead. + +```dart +class HeroService {} + +const someProviders = const [ + const ClassProvider(HeroService), +]; +``` + ## Tokens ### DO use typed `OpaqueToken` From a6ce599c07795d709b6d16c3d93f696f54479b20 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Fri, 2 Mar 2018 15:15:10 -0800 Subject: [PATCH 5/5] More updates. --- doc/effective/di.md | 108 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 107 insertions(+), 1 deletion(-) diff --git a/doc/effective/di.md b/doc/effective/di.md index 04917eaab2..aea4d21a90 100644 --- a/doc/effective/di.md +++ b/doc/effective/di.md @@ -16,6 +16,10 @@ There are many different ways to use AngularDart to provide and utilize dependen * [PREFER `MultiToken` to `multi: true`](#...) * [Injectors](#injectors) * [AVOID using `ReflectiveInjector`](#avoid-using-reflectiveinjector) +* [Components](#components) + * [AVOID using injection to configure individual components](#...) +* [Annotations](#annotations) + * [PREFER omitting `@Injectable()` where possible](#...) ## Providers @@ -316,4 +320,106 @@ import 'file.template.dart' as ng; final InjectorFactory heroInjector = ng.heroInjector$Injector; ``` -## Components \ No newline at end of file +## Components + +### AVOID using injection to configure individual components + +Imagine you have a component (`HeroComponent`, or ``) that requires a model object (`HeroData`) in order to render. You could use `@Input()` _or_ could use dependency injection. + +```dart +Class HeroData { + final bool hasPowerOfFlight; + final bool vulnerableToGreenGlowingRocks; + + HeroData({ + this.hasPowerOfFlight: false, + this.vulnerableToGreenGlowingRocks: false, + }); +} +``` + +**BAD**: Using dependency injection for configuration. + +```dart +@Component( + selector: 'hero-component', + template: 'I am a HERO, but can I fly: {{canIFly}}', +) +class HeroComponent { + final HeroData _heroData; + + HeroComponent(this._heroData); + + String get canIFly { + return _heroData.hasPowerOfFlight ? 'Yes' : 'No'; + } +} +``` + +While this might _look_ pleasing (i.e. for testing), it means it becomes very verbose, confusing, and difficult to have multiple hero components with different configurations - probably not what you want. Use `@Input` instead. + +**GOOD**: Using `@Input()` instead for configuration. + +```dart +@Component( + selector: 'hero-component', + template: 'I am a HERO, but can I fly: {{canIFly}}', +) +class HeroComponent { + @Input('can-fly') + bool hasPowerOfFlight = false; + + String get canIFly { + return hasPowerOfFlight ? 'Yes' : 'No'; + } +} +``` + +**GOOD**: Using dependency injection when configuring app-wide components. + +If you want to, for example, set a property on _all_ `HeroComponent`s, then dependency injection is perfectly fine (and preferred if you will have many of them). You could also consider using `@Optional()` in order to avoid making it a required service. + +```dart +abstract class HeroAuthentication { + bool isLoggedInAsHero(String name); +} + +@Component( + selector: 'hero-component', + template: '...', +) +class HeroComponent { + final HeroAuthentication _auth; + + HeroComponent(@Optional() this._auth); + + @Input() + set name(String name) { + if (_auth != null && !_auth.isLoggedInAsHero(name)) { + throw new ArgumentError('Not logged in!'); + } + // ... + } +} +``` + +## Annotations + +### PREFER omitting `@Injectable()` where possible + +While sometimes not clear, the `@Injectable()` annotation is actually a legacy feature that is required for runtime-configuration of injectors. + +> **NOTE**: For teams or users still utilizing the `bootstrapStatic` (most users as of this writing) method or creating `ReflectiveInjector`s at runtime, _disregard this part of the guide_, but also know that this pattern is not desirable long-term - it increases the code-size of your app somewhat dramatically. + +**BAD**: + +```dart +@Injectable() +class HeroService {} +``` + +**GOOD**: + +```dart +class HeroService {} +``` \ No newline at end of file