Skip to content
This repository has been archived by the owner on Sep 16, 2022. It is now read-only.

Commit

Permalink
feat(Compiler): new X(injectorGet(Y)) finally traces to "X -> Y".
Browse files Browse the repository at this point in the history 
Partially closes #434 (still need factories).

Downsides of this implementation is mostly the code it emits now for traceable `new X(...)` is plain ugly, though we wouldn't need the worst part, the ternary expression, if dart-lang/sdk#34141 was addressed.

Ideally we would not use this wrapper expression at all, but it is currently NP-hard to refactor what is an expression into a statement due to how the compiler pipeline works (the expression is created before the assignment), so this seems like the least-bad option _if_ we want this fix.

Also deleted some unused code.

Closes #1573

PiperOrigin-RevId: 209010949
  • Loading branch information
@matanlurey
matanlurey committed Aug 16, 2018
1 parent d88d883 commit 53b4773
Show file tree
Hide file tree
Showing 16 changed files with 161 additions and 181 deletions.
8 changes: 5 additions & 3 deletions _goldens/test/_files/dart2js/dart2js_golden.golden
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9419,10 +9419,12 @@
t2 = this.parentView;
t4 = t2.injectorGet$2(C.MultiToken_usPresidents, this.viewData.parentIndex);
t5 = t2.injectorGet$2(C.MultiToken_whiteHouse, this.viewData.parentIndex);
t2 = t2.injectorGet$2(C.Type_InjectableService_AmT, this.viewData.parentIndex);
t6 = new E.InjectsFromArbitraryParent();
t2.injectorGet$2(C.Type_InjectableService_AmT, this.viewData.parentIndex).printWashingtonDc$2(t5, t4);
this._InjectsFromArbitraryParent_3_5 = t6;
this._compView_3.create$2(0, t6, []);
t2.printWashingtonDc$2(t5, t4);
t2 = t6;
this._InjectsFromArbitraryParent_3_5 = t2;
this._compView_3.create$2(0, t2, []);
t1 = new N.ViewComponentConditionalFeatures0(P.LinkedHashMap_LinkedHashMap$_empty(t1, null), this);
t1.viewData = S.AppViewData_AppViewData(t1, 3, C.ViewType_1, 4);
t2 = t3.createElement("component-conditional-features");
Expand Down
15 changes: 12 additions & 3 deletions _goldens/test/_files/dart2js/dart2js_golden.template.golden
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ import 'package:angular/src/core/change_detection/change_detection.dart';
import 'package:angular/src/core/linker/app_view_utils.dart' as import6;
import 'package:angular/src/runtime.dart' as import7;
import 'package:angular/angular.dart';
import 'package:angular/src/core/di/opaque_token.dart' as import9;
import 'package:angular/src/di/errors.dart' as import9;
import 'package:angular/src/core/di/opaque_token.dart' as import10;
import 'dart:core';
import 'package:angular/src/core/linker/view_container.dart';
import 'package:angular/src/common/directives/ng_if.dart';
Expand Down Expand Up @@ -71,7 +72,11 @@ class ViewRootComponent0 extends AppView<import1.RootComponent> {
_compView_3 = ViewInjectsFromArbitraryParent0(this, 3);
_el_3 = _compView_3.rootEl;
parentRenderNode.append(_el_3);
_InjectsFromArbitraryParent_3_5 = import1.InjectsFromArbitraryParent(parentView.injectorGet(const import9.MultiToken<String>('usPresidents'), viewData.parentIndex), parentView.injectorGet(const import9.MultiToken<String>('whiteHouse'), viewData.parentIndex), parentView.injectorGet(import1.InjectableService, viewData.parentIndex));
_InjectsFromArbitraryParent_3_5 = (import7.isDevMode
? import9.debugInjectorWrap(import1.InjectsFromArbitraryParent, () {
return import1.InjectsFromArbitraryParent(parentView.injectorGet(const import10.MultiToken<String>('usPresidents'), viewData.parentIndex), parentView.injectorGet(const import10.MultiToken<String>('whiteHouse'), viewData.parentIndex), parentView.injectorGet(import1.InjectableService, viewData.parentIndex));
})
: import1.InjectsFromArbitraryParent(parentView.injectorGet(const import10.MultiToken<String>('usPresidents'), viewData.parentIndex), parentView.injectorGet(const import10.MultiToken<String>('whiteHouse'), viewData.parentIndex), parentView.injectorGet(import1.InjectableService, viewData.parentIndex)));
_compView_3.create(_InjectsFromArbitraryParent_3_5, []);
_compView_4 = ViewComponentConditionalFeatures0(this, 4);
_el_4 = _compView_4.rootEl;
Expand Down Expand Up @@ -573,7 +578,11 @@ class _ViewInjectsFromArbitraryParentHost0 extends AppView<import1.InjectsFromAr
ComponentRef<import1.InjectsFromArbitraryParent> build() {
_compView_0 = ViewInjectsFromArbitraryParent0(this, 0);
rootEl = _compView_0.rootEl;
_InjectsFromArbitraryParent_0_5 = import1.InjectsFromArbitraryParent(this.injectorGet(const import9.MultiToken<String>('usPresidents'), viewData.parentIndex), this.injectorGet(const import9.MultiToken<String>('whiteHouse'), viewData.parentIndex), this.injectorGet(import1.InjectableService, viewData.parentIndex));
_InjectsFromArbitraryParent_0_5 = (import7.isDevMode
? import9.debugInjectorWrap(import1.InjectsFromArbitraryParent, () {
return import1.InjectsFromArbitraryParent(this.injectorGet(const import10.MultiToken<String>('usPresidents'), viewData.parentIndex), this.injectorGet(const import10.MultiToken<String>('whiteHouse'), viewData.parentIndex), this.injectorGet(import1.InjectableService, viewData.parentIndex));
})
: import1.InjectsFromArbitraryParent(this.injectorGet(const import10.MultiToken<String>('usPresidents'), viewData.parentIndex), this.injectorGet(const import10.MultiToken<String>('whiteHouse'), viewData.parentIndex), this.injectorGet(import1.InjectableService, viewData.parentIndex)));
_compView_0.create(_InjectsFromArbitraryParent_0_5, projectableNodes);
init0(rootEl);
return ComponentRef(0, this, rootEl, _InjectsFromArbitraryParent_0_5);
Expand Down
11 changes: 8 additions & 3 deletions _goldens/test/_files/injectables.template.golden
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@ import 'package:angular/src/core/change_detection/change_detection.dart';
import 'package:angular/src/core/linker/app_view_utils.dart' as import6;
import 'package:angular/src/runtime.dart' as import7;
import 'package:angular/angular.dart';
import 'package:angular/src/di/errors.dart' as import9;
import 'dart:core';
import 'package:angular/src/core/di/opaque_token.dart' as import10;
import 'package:angular/src/core/di/opaque_token.dart' as import11;

final List<dynamic> styles$InjectableComponent = const [];

Expand Down Expand Up @@ -78,15 +79,19 @@ class _ViewInjectableComponentHost0 extends AppView<import1.InjectableComponent>
_compView_0 = ViewInjectableComponent0(this, 0);
rootEl = _compView_0.rootEl;
_test_0_5 = import1.injectableFactory(this.injectorGet(import2.Window, viewData.parentIndex));
_InjectableComponent_0_6 = import1.InjectableComponent(null, this.injectorGet(import1.InjectableService, viewData.parentIndex, null), _test_0_5, this.injectorGet(String, viewData.parentIndex, null));
_InjectableComponent_0_6 = (import7.isDevMode
? import9.debugInjectorWrap(import1.InjectableComponent, () {
return import1.InjectableComponent(null, this.injectorGet(import1.InjectableService, viewData.parentIndex, null), _test_0_5, this.injectorGet(String, viewData.parentIndex, null));
})
: import1.InjectableComponent(null, this.injectorGet(import1.InjectableService, viewData.parentIndex, null), _test_0_5, this.injectorGet(String, viewData.parentIndex, null)));
_compView_0.create(_InjectableComponent_0_6, projectableNodes);
init0(rootEl);
return ComponentRef(0, this, rootEl, _InjectableComponent_0_6);
}

@override
dynamic injectorGetInternal(dynamic token, int nodeIndex, dynamic notFoundResult) {
if ((identical(token, const import10.OpaqueToken('test')) && (0 == nodeIndex))) {
if ((identical(token, const import11.OpaqueToken('test')) && (0 == nodeIndex))) {
return _test_0_5;
}
if ((identical(token, import1.SomeDep) && (0 == nodeIndex))) {
Expand Down
13 changes: 11 additions & 2 deletions _goldens/test/_files/opaque_token.template.golden
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import 'package:angular/src/core/di/opaque_token.dart' as import9;
import 'package:angular/src/core/linker/view_container.dart';
import 'package:angular/src/common/directives/ng_if.dart';
import 'package:angular/src/core/linker/template_ref.dart';
import 'package:angular/src/di/errors.dart' as import13;
import 'dart:core';

final List<dynamic> styles$HasOpaqueTokens = const [];
Expand Down Expand Up @@ -187,7 +188,11 @@ class _ViewContainsChildComponent2 extends AppView<import1.ContainsChildComponen
_compView_1 = ViewInjectsTypedTokenFromSomeParent0(this, 1);
_el_1 = _compView_1.rootEl;
_el_0.append(_el_1);
_InjectsTypedTokenFromSomeParent_1_5 = import1.InjectsTypedTokenFromSomeParent(parentView.parentView.injectorGet(const import9.OpaqueToken<List<Duration>>('listOfDuration'), parentView.viewData.parentIndex));
_InjectsTypedTokenFromSomeParent_1_5 = (import7.isDevMode
? import13.debugInjectorWrap(import1.InjectsTypedTokenFromSomeParent, () {
return import1.InjectsTypedTokenFromSomeParent(parentView.parentView.injectorGet(const import9.OpaqueToken<List<Duration>>('listOfDuration'), parentView.viewData.parentIndex));
})
: import1.InjectsTypedTokenFromSomeParent(parentView.parentView.injectorGet(const import9.OpaqueToken<List<Duration>>('listOfDuration'), parentView.viewData.parentIndex)));
_compView_1.create(_InjectsTypedTokenFromSomeParent_1_5, []);
init0(_el_0);
return null;
Expand Down Expand Up @@ -276,7 +281,11 @@ class _ViewInjectsTypedTokenFromSomeParentHost0 extends AppView<import1.InjectsT
ComponentRef<import1.InjectsTypedTokenFromSomeParent> build() {
_compView_0 = ViewInjectsTypedTokenFromSomeParent0(this, 0);
rootEl = _compView_0.rootEl;
_InjectsTypedTokenFromSomeParent_0_5 = import1.InjectsTypedTokenFromSomeParent(this.injectorGet(const import9.OpaqueToken<List<Duration>>('listOfDuration'), viewData.parentIndex));
_InjectsTypedTokenFromSomeParent_0_5 = (import7.isDevMode
? import13.debugInjectorWrap(import1.InjectsTypedTokenFromSomeParent, () {
return import1.InjectsTypedTokenFromSomeParent(this.injectorGet(const import9.OpaqueToken<List<Duration>>('listOfDuration'), viewData.parentIndex));
})
: import1.InjectsTypedTokenFromSomeParent(this.injectorGet(const import9.OpaqueToken<List<Duration>>('listOfDuration'), viewData.parentIndex)));
_compView_0.create(_InjectsTypedTokenFromSomeParent_0_5, projectableNodes);
init0(rootEl);
return ComponentRef(0, this, rootEl, _InjectsTypedTokenFromSomeParent_0_5);
Expand Down
19 changes: 16 additions & 3 deletions _goldens/test/_files/visibility.template.golden
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import 'dart:html' as import5;
import 'package:angular/src/core/linker/app_view_utils.dart' as import6;
import 'package:angular/src/runtime.dart' as import7;
import 'package:angular/angular.dart';
import 'package:angular/src/di/errors.dart' as import9;

final List<dynamic> styles$Dependent = const [];

Expand Down Expand Up @@ -50,7 +51,11 @@ class _ViewDependentHost0 extends AppView<import1.Dependent> {
ComponentRef<import1.Dependent> build() {
_compView_0 = ViewDependent0(this, 0);
rootEl = _compView_0.rootEl;
_Dependent_0_5 = import1.Dependent(this.injectorGet(import1.Dependency, viewData.parentIndex));
_Dependent_0_5 = (import7.isDevMode
? import9.debugInjectorWrap(import1.Dependent, () {
return import1.Dependent(this.injectorGet(import1.Dependency, viewData.parentIndex));
})
: import1.Dependent(this.injectorGet(import1.Dependency, viewData.parentIndex)));
_compView_0.create(_Dependent_0_5, projectableNodes);
init0(rootEl);
return ComponentRef(0, this, rootEl, _Dependent_0_5);
Expand Down Expand Up @@ -95,7 +100,11 @@ class ViewDependencyWithDependentInView0 extends AppView<import1.DependencyWithD
_compView_0 = ViewDependent0(this, 0);
_el_0 = _compView_0.rootEl;
parentRenderNode.append(_el_0);
_Dependent_0_5 = import1.Dependent(parentView.injectorGet(import1.Dependency, viewData.parentIndex));
_Dependent_0_5 = (import7.isDevMode
? import9.debugInjectorWrap(import1.Dependent, () {
return import1.Dependent(parentView.injectorGet(import1.Dependency, viewData.parentIndex));
})
: import1.Dependent(parentView.injectorGet(import1.Dependency, viewData.parentIndex)));
_compView_0.create(_Dependent_0_5, []);
init(const [], null);
return null;
Expand Down Expand Up @@ -249,7 +258,11 @@ class ViewDependencyAndDependentInView0 extends AppView<import1.DependencyAndDep
_compView_1 = ViewDependent0(this, 1);
_el_1 = _compView_1.rootEl;
_el_0.append(_el_1);
_Dependent_1_5 = import1.Dependent(parentView.injectorGet(import1.Dependency, viewData.parentIndex));
_Dependent_1_5 = (import7.isDevMode
? import9.debugInjectorWrap(import1.Dependent, () {
return import1.Dependent(parentView.injectorGet(import1.Dependency, viewData.parentIndex));
})
: import1.Dependent(parentView.injectorGet(import1.Dependency, viewData.parentIndex)));
_compView_1.create(_Dependent_1_5, []);
init(const [], null);
return null;
Expand Down
55 changes: 11 additions & 44 deletions _tests/test/di/directive_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,99 +158,65 @@ void main() {
() => testBed.create(),
throwsA(
predicate(
(e) => '$e'.endsWith('No provider found for $MissingService'),
(e) => '$e'.contains('No provider found for $MissingService'),
),
),
);
});

test('should throw a readable error message on a 2-node failure', () {
// NOTE: In an ideal scenario, this would throw a better error, i.e.
// InjectsMissingService -> MissingService
//
// ... but this would require enter() and leave() wrapping around the
// successful cases in AppView-local injection (and changes to the
// generated code).
//
// If we end up doing this, we should modify the test accordingly.
final testBed = NgTestBed<WillFailInjecting2Node>();
expect(
() => testBed.create(),
throwsA(
predicate(
(e) => '$e'.endsWith('No provider found for $MissingService'),
(e) => '$e'.contains('No provider found for $MissingService: '
'$InjectsMissingService -> $MissingService'),
),
),
reason: 'View compiler does not trace local injections (#434)',
);
});

test('should throw a readable error message on a child directive', () {
// NOTE: In an ideal scenario, this would throw a better error, i.e.
// WillFailInjecting1NodeParent -> MissingService
//
// ... but this would require enter() and leave() wrapping around the
// successful cases in AppView-local injection (and changes to the
// generated code).
//
// If we end up doing this, we should modify the test accordingly.
final testBed = NgTestBed<WillFailCreatingChild>();
expect(
() => testBed.create(),
throwsA(
predicate(
(e) => '$e'.endsWith('No provider found for $MissingService'),
(e) => '$e'.contains('No provider found for $MissingService: '
'$WillFailInjecting1Node -> $MissingService'),
),
),
reason: 'View compiler does not trace local injections (#434)',
);
});

test('should throw a readable error message in an embedded template', () {
// NOTE: In an ideal scenario, this would throw a better error, i.e.
// WillFailInjecting1NodeParent -> MissingService
//
// ... but this would require enter() and leave() wrapping around the
// successful cases in AppView-local injection (and changes to the
// generated code).
//
// If we end up doing this, we should modify the test accordingly.
final testBed = NgTestBed<WillFailCreatingChildInTemplate>();
expect(
() => testBed.create(),
throwsA(
predicate(
(e) => '$e'.endsWith('No provider found for $MissingService'),
(e) => '$e'.contains('No provider found for $MissingService: '
'$WillFailInjecting1Node -> $MissingService'),
),
),
reason: 'View compiler does not trace local injections (#434)',
);
});

test('should throw a readable error message when quering a child', () {
// NOTE: In an ideal scenario, this would throw a better error, i.e.
// InjectsMissingService -> MissingService
//
// ... but this would require enter() and leave() wrapping around the
// successful cases in AppView-local injection (and changes to the
// generated code).
//
// If we end up doing this, we should modify the test accordingly.
final testBed = NgTestBed<WillFailQueryingServiceInTemplate>();
expect(
() => testBed.create(),
throwsA(
predicate(
(e) => '$e'.endsWith('No provider found for $MissingService'),
(e) => '$e'.contains('No provider found for $MissingService: '
'$InjectsMissingService -> $MissingService'),
),
),
reason: 'View compiler does not trace local injections (#434)',
);
});

test('should throw a readable error message on a 2-node/parent failure', () {
// Passes, unlike the missing error case, because the parent injector, in
// this case a ReflectiveInjector, *does* trace the individual calls.
final testBed = NgTestBed<WillFailInjecting2NodeParent>().addProviders([
Provider(
InjectsMissingService,
Expand All @@ -266,7 +232,8 @@ void main() {
predicate(
(e) => '$e'.contains(''
'No provider found for $MissingService: '
'$InjectsMissingService -> $MissingService.'),
'$WillFailInjecting2NodeParent -> $InjectsMissingService -> '
'$MissingService.'),
),
),
);
Expand Down
7 changes: 7 additions & 0 deletions angular/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,13 @@
as `null` (either intentionally, or due to analysis errors/imports missing)
a better error message is now thrown with the context of the error.
* [#434][]: In development mode, creating a component or service `C` that
attempts to inject a missing dynamic dependency `D` will now throw an error
message containing `Could not find provider for D: C -> D`. Previously the
message was only `Could not find provider for D` in these cases, which was
often not enough information to debug easily.
[#434]: https://github.com/dart-lang/angular/issues/434
[#880]: https://github.com/dart-lang/angular/issues/880
[#1538]: https://github.com/dart-lang/angular/issues/1538
[#1539]: https://github.com/dart-lang/angular/issues/1539
Expand Down
2 changes: 2 additions & 0 deletions angular/lib/src/compiler/identifiers.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,8 @@ class Identifiers {
name: "debugInjectorEnter", moduleUrl: debugInjectorModuleUrl);
static final debugInjectorLeave = CompileIdentifierMetadata<dynamic>(
name: "debugInjectorLeave", moduleUrl: debugInjectorModuleUrl);
static final debugInjectorWrap = CompileIdentifierMetadata<dynamic>(
name: "debugInjectorWrap", moduleUrl: debugInjectorModuleUrl);

static final interpolate = <CompileIdentifierMetadata>[
CompileIdentifierMetadata<dynamic>(
Expand Down
20 changes: 16 additions & 4 deletions angular/lib/src/compiler/view_compiler/compile_element.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -418,21 +418,33 @@ class CompileElement extends CompileNode implements ProvidersNodeHost {
}

@override
ProviderSource createDynamicInjectionSource(ProvidersNode providersNode,
ProviderSource source, CompileTokenMetadata token, bool optional) {
ProviderSource createDynamicInjectionSource(
ProvidersNode providersNode,
ProviderSource source,
CompileTokenMetadata token,
bool optional,
) {
// If request was made on a service resolving to a private directive,
// use requested dependency to call injectorGet instead of directive
// that redirects using useExisting type provider.
var value = source?.build();
value ??= injectFromViewParentInjector(view, token, optional);
var hasDynamicDependencies = false;
if (value == null) {
value = injectFromViewParentInjector(view, token, optional);
hasDynamicDependencies = true;
}
CompileElement currElement = this;
while (currElement != null) {
if (currElement._providers == providersNode) break;
currElement = currElement.parent;
}
var viewRelativeExpression =
getPropertyInView(value, view, currElement.view);
return LiteralValueSource(token, viewRelativeExpression);
return LiteralValueSource(
token,
viewRelativeExpression,
hasDynamicDependencies: hasDynamicDependencies,
);
}

List<o.Expression> getProviderTokens() {
Expand Down
Loading

0 comments on commit 53b4773

Please sign in to comment.