Hello.
We have discovered a very disappointing behavior of the dart tree shaking in case of the angular-annotated classes.
As you can see, there is no usage of any InjectableService or SimpleService in the code below:
/// my_servises.dart
import 'package:angular/di.dart';
@Injectable()
class InjectableService {
String myProperty = 'Wubalubadubdub';
}
class SimpleService {
String myProperty = 'Good news, everyone!';
}
/// my_cmp.dart
import 'src/injectable_service.dart';
@Component(
selector: 'my-app',
template: '<div></div>'
)
class AppComponent { }
If we build that project with pub build and look into the compiled main.dart.js we will see that the string 'Good news, everyone!' is not present there, which is definitely good news, because that shows us expected behavior of a tree shaking algorithm.
However, the string 'Wubalubadubdub' was not tree shacked at all. The same behavior is true for any @Component class and all its methods or properties; as well as it is true for library files that export bunch of services or components. Like this:
// this_service_is_not_used_by_anyone.dart;
@Injectable()
class InjectableService {}
/// services.dart
export 'this_service_is_not_used_by_anyone.dart';
export 'and_this_service_too.dart';
export 'but_they_will_eventually_be_added_to_the_build_file.dart';
/// my_cmp.dart
import 'src/services.dart';
@Component(...)
class AppComponent { }
The situation is even worse if we DO use a class type, but do not use its methods or properties:
/// my_cmp.dart
import 'src/services.dart' show InjectableService;
@Component(
selector: 'my-app',
template: '<div></div>',
providers: const [InjectableService]
)
class AppComponent { }
In that case, dart analyzer will be unable to track that situation because an import is now used.
@AndreyChernykh:
We have discovered a very disappointing behavior of the dart tree shaking in case of the angular-annotated classes.
As you can see, there is no usage of any InjectableService or SimpleService in the code below:
/// my_servises.dart import 'package:angular/di.dart'; @Injectable() class InjectableService { String myProperty = 'Wubalubadubdub'; } class SimpleService { String myProperty = 'Good news, everyone!'; } /// my_cmp.dart import 'src/injectable_service.dart'; @Component( selector: 'my-app', template: '<div></div>' ) class AppComponent { }
If we build that project with
pub buildand look into the compiledmain.dart.jswe will see that the string'Good news, everyone!'is not present there, which is definitely good news, because that shows us expected behavior of a tree shaking algorithm.
However, the string
'Wubalubadubdub'was not tree shacked at all. The same behavior is true for any@Componentclass and all its methods or properties; as well as it is true for library files that export bunch of services or components.
So this is a known (set of) issue(s).
Historically, every annotation was runtime processed when we had a dart:mirrors-based implementation and a "code generation"-based implementation - they shared quite a bit of code, and as a result, were the lowest common denominator.
We've removed dart:mirrors, and fixed quite a bit since then. As of 4.x.x, we didn't quite get to fix _everything_, there are still two major "holes" in the framework that negatively effect tree-shaking:
SlowComponentLoaderReflectiveInjector... both of these rely on annotated classes not being tree-shaken, unfortunately.
The former is replaceable with the tree-shakeable ComponentLoader API, but the latter doesn't have a replacement (yet) - there is WIP to have a static Injector generator that doesn't rely on the ReflectiveInjector API at runtime.
A goal of 5.x.x is to be "fast by default", but it also would mean potentially lots of breaking changes, so you'll want to follow the DI 1.5 and DI 2.0 milestones.
You can see some (unsupported) experimental APIs that attempt to start changing that at HEAD:
https://github.com/dart-lang/angular/blob/master/angular/lib/experimental.dart
/// Bootstrap a new AngularDart application.
///
/// Uses a pre-compiled [factory] as the root component, instead of looking up
/// via [Type] at runtime, which requires `initReflector`. May optionally define
/// root-level services by providing a [createAppInjector].
///
/// **WARNING**: This API is not considered part of the stable API.
@experimental
ComponentRef<T> bootstrapFactory<T>(
ComponentFactory<T> factory, [
Injector createAppInjector(Injector parent),
]) {
final appInjector = createAppInjector == null
? _appInjector(_platformInjector)
: createAppInjector(_appInjector(_platformInjector));
initAngular(appInjector);
final appRef = appInjector.get(ApplicationRef) as ApplicationRef;
return appRef.bootstrap(factory);
}
The situation is even worse if we DO use a class type, but do not use its methods or properties:
/// my_cmp.dart import 'src/services.dart' show InjectableService; @Component( selector: 'my-app', template: '<div></div>', providers: const [InjectableService] ) class AppComponent { }In that case, dart analyzer will be unable to track that situation because an
importis now used.
You're right, but we would require a whole-program analysis step to be able to conclusively know InjectableService is not used. For example, someone could write this API:
void injectFoo(Injector injector, dynamic service) {
injector.get(service);
}
... so while it would be trivial to add a heuristic for this simple case, it wouldn't really help most users. Consider the following case:
class ParentOfApp implements OnInit {
@ViewChild(AppComponent, read: Injector) Injector injector;
@override
ngOnInit() {
injector.get(InjectableService);
}
}
... this is totally valid, but we couldn't know just looking at AppComponent.
Thank you for the explanation.
Just an update. This is almost complete; the feature technically exists now, but was experimental pending some validation. I think we're going to ship it in an upcoming alpha release, the API just needs some cleanup.
This is complete as of HEAD. We'll have CHANGELOG entries/docs on this soon.
Most helpful comment
Just an update. This is almost complete; the feature technically exists now, but was experimental pending some validation. I think we're going to ship it in an upcoming alpha release, the API just needs some cleanup.