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

Commit

Permalink
chore(Test): Add more tests, some refactoring to ViewCompiler DI
Browse files Browse the repository at this point in the history
Partial work towards #434.

It looks like unfortunately completing this fix might be [virtually] infeasible without dart-lang/sdk#34141, though we could potentially use a ternary expression (dart-lang/sdk#34141 (comment)).

Closes #1568

PiperOrigin-RevId: 208663946
  • Loading branch information
matanlurey committed Aug 14, 2018
1 parent e756114 commit b9f817e
Show file tree
Hide file tree
Showing 7 changed files with 175 additions and 27 deletions.
109 changes: 107 additions & 2 deletions _tests/test/di/directive_test.dart
Expand Up @@ -181,11 +181,74 @@ void main() {
(e) => '$e'.endsWith('No provider found for $MissingService'),
),
),
reason: 'AppView does not trace local injections',
reason: 'View compiler does not trace local injections (#434)',
);
});

test('should throw a readable erro message on a 2-node/parent failure', () {
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'),
),
),
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'),
),
),
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'),
),
),
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([
Expand Down Expand Up @@ -613,6 +676,48 @@ class WillFailInjecting2NodeParent {
WillFailInjecting2NodeParent(InjectsMissingService _);
}

@Component(
selector: 'will-fail-creating-child',
template: r'''
<will-fail-injecting-1-node></will-fail-injecting-1-node>
''',
directives: [WillFailInjecting1Node],
)
class WillFailCreatingChild {}

@Component(
selector: 'will-fail-creating-child',
template: r'''
<will-fail-injecting-1-node *ngIf="showChild"></will-fail-injecting-1-node>
''',
directives: [
NgIf,
WillFailInjecting1Node,
],
)
class WillFailCreatingChildInTemplate {
var showChild = true;
}

@Component(
selector: 'will-fail-querying-service',
template: r'''
<lazy-provides-missing-service></lazy-provides-missing-service>
''',
directives: [
LazilyProvidesMissingService,
],
)
class WillFailQueryingServiceInTemplate {
@ViewChild(LazilyProvidesMissingService, read: InjectsMissingService)
InjectsMissingService willFailDuringQuery;
}

@Directive(selector: 'lazy-provides-missing-service', providers: [
InjectsMissingService,
])
class LazilyProvidesMissingService {}

const baseUrl = OpaqueToken<String>('baseUrl');

@Component(
Expand Down
6 changes: 5 additions & 1 deletion angular/lib/src/compiler/identifiers.dart
Expand Up @@ -2,7 +2,6 @@ import "compile_metadata.dart"
show CompileIdentifierMetadata, CompileTokenMetadata;

final appViewModuleUrl = "asset:angular/lib/src/core/linker/app_view.dart";
final debugAppViewModuleUrl = "asset:angular/lib/src/debug/debug_app_view.dart";
final appViewUtilsModuleUrl =
"asset:angular/lib/src/core/linker/app_view_utils.dart";
final cdModuleUrl =
Expand All @@ -13,6 +12,7 @@ final ngForUrl = "asset:angular/lib/src/common/directives/ng_for.dart";
final profileRuntimeModuleUrl =
"asset:angular/lib/src/debug/profile_runtime.dart";
final runtimeUtilsModuleUrl = "asset:angular/lib/src/runtime.dart";
final debugInjectorModuleUrl = 'asset:angular/lib/src/di/errors.dart';

class Identifiers {
static final appViewUtils = CompileIdentifierMetadata<dynamic>(
Expand Down Expand Up @@ -106,6 +106,10 @@ class Identifiers {
name: "isDevMode", moduleUrl: runtimeUtilsModuleUrl);
static final unsafeCast = CompileIdentifierMetadata<dynamic>(
name: "unsafeCast", moduleUrl: runtimeUtilsModuleUrl);
static final debugInjectorEnter = CompileIdentifierMetadata<dynamic>(
name: "debugInjectorEnter", moduleUrl: debugInjectorModuleUrl);
static final debugInjectorLeave = CompileIdentifierMetadata<dynamic>(
name: "debugInjectorLeave", moduleUrl: debugInjectorModuleUrl);

static final interpolate = <CompileIdentifierMetadata>[
CompileIdentifierMetadata<dynamic>(
Expand Down
8 changes: 6 additions & 2 deletions angular/lib/src/compiler/view_compiler/event_binder.dart
Expand Up @@ -61,8 +61,12 @@ class CompileEventListener {
_eventParam = o.FnParam(EventHandlerVars.event.name, o.importType(null));
}

void _addAction(BoundEventAst hostEvent, CompileDirectiveMetadata directive,
ProviderSource directiveInstance, AnalyzedClass clazz) {
void _addAction(
BoundEventAst hostEvent,
CompileDirectiveMetadata directive,
ProviderSource directiveInstance,
AnalyzedClass clazz,
) {
if (_isTearoff(hostEvent)) {
hostEvent = _rewriteTearoff(hostEvent, clazz);
}
Expand Down
10 changes: 10 additions & 0 deletions angular/lib/src/compiler/view_compiler/ir/provider_source.dart
Expand Up @@ -10,7 +10,17 @@ abstract class ProviderSource {
final CompileTokenMetadata token;
final bool eager;
final bool multiProvider;

ProviderSource(this.token, {this.eager, this.multiProvider});

o.Expression build();

/// Whether a dynamic `injectorGet(...)` is required to resolve this provider.
///
/// For example:
/// ```dart
/// // DependencyService is dynamically required to resolve MyService.
/// _MyService = MyService(injectorGet(DependencyService));
/// ```
bool get hasDynamicDependencies;
}
53 changes: 43 additions & 10 deletions angular/lib/src/compiler/view_compiler/ir/providers_node.dart
Expand Up @@ -238,24 +238,42 @@ abstract class ProvidersNodeHost {
}

class BuiltInSource extends ProviderSource {
o.Expression _value;
final o.Expression _value;

BuiltInSource(CompileTokenMetadata token, this._value) : super(token);

@override
o.Expression build() => _value;

@override
final hasDynamicDependencies = false;
}

class LiteralValueSource extends ProviderSource {
o.Expression _value;
final o.Expression _value;

LiteralValueSource(CompileTokenMetadata token, this._value) : super(token);

@override
o.Expression build() => _value;

@override
final hasDynamicDependencies = false;
}

bool _hasDynamicDependencies(Iterable<ProviderSource> sources) {
for (final source in sources) {
if (source is DynamicProviderSource || source.hasDynamicDependencies) {
return true;
}
}
return false;
}

class FactoryProviderSource extends ProviderSource {
CompileFactoryMetadata _factory;
List<ProviderSource> _parameters;
final CompileFactoryMetadata _factory;
final List<ProviderSource> _parameters;

FactoryProviderSource(
CompileTokenMetadata token, this._factory, this._parameters)
: super(token);
Expand All @@ -266,12 +284,15 @@ class FactoryProviderSource extends ProviderSource {
for (ProviderSource s in _parameters) paramExpressions.add(s.build());
return o.importExpr(_factory).callFn(paramExpressions);
}

@override
bool get hasDynamicDependencies => _hasDynamicDependencies(_parameters);
}

class ClassProviderSource extends ProviderSource {
CompileTypeMetadata _classType;
List<ProviderSource> _parameters;
List<o.OutputType> _typeArguments;
final CompileTypeMetadata _classType;
final List<ProviderSource> _parameters;
final List<o.OutputType> _typeArguments;

ClassProviderSource(
CompileTokenMetadata token,
Expand All @@ -288,11 +309,15 @@ class ClassProviderSource extends ProviderSource {
return o.importExpr(_classType).instantiate(paramExpressions,
type: o.importType(_classType), genericTypes: _typeArguments);
}

@override
bool get hasDynamicDependencies => _hasDynamicDependencies(_parameters);
}

class FunctionalDirectiveSource extends ProviderSource {
CompileTypeMetadata _classType;
List<ProviderSource> _parameters;
final CompileTypeMetadata _classType;
final List<ProviderSource> _parameters;

FunctionalDirectiveSource(
CompileTokenMetadata token, this._classType, this._parameters)
: super(token);
Expand All @@ -303,14 +328,18 @@ class FunctionalDirectiveSource extends ProviderSource {
for (ProviderSource s in _parameters) paramExpressions.add(s.build());
return o.importExpr(_classType).callFn(paramExpressions);
}

@override
bool get hasDynamicDependencies => _hasDynamicDependencies(_parameters);
}

/// Source for injectable values that are resolved by
/// dynamic lookup (injectorGet).
class DynamicProviderSource extends ProviderSource {
ProvidersNode parentProviders;
final ProvidersNode parentProviders;
final bool optional;
final bool _isAppViewHost;

DynamicProviderSource(this.parentProviders, CompileTokenMetadata token,
this.optional, this._isAppViewHost)
: super(token);
Expand All @@ -328,4 +357,8 @@ class DynamicProviderSource extends ProviderSource {
}
return viewExpr.callMethod('injectorGet', args);
}

// It *might*, but we don't know.
@override
final hasDynamicDependencies = false;
}
Expand Up @@ -91,7 +91,10 @@ o.Expression getPropertyInView(
}

o.Expression injectFromViewParentInjector(
CompileView view, CompileTokenMetadata token, bool optional) {
CompileView view,
CompileTokenMetadata token,
bool optional,
) {
o.Expression viewExpr = (view.viewType == ViewType.host)
? o.THIS_EXPR
: o.ReadClassMemberExpr('parentView');
Expand Down
11 changes: 0 additions & 11 deletions angular/lib/src/di/errors.dart
@@ -1,5 +1,4 @@
import 'package:angular/src/runtime.dart';
import 'package:meta/meta.dart';

/// Current stack of tokens being requested for an injection.
List<Object> _tokenStack;
Expand Down Expand Up @@ -33,10 +32,6 @@ void debugInjectorLeave(Object token) {
if (!isDevMode) {
return;
}
// Don't affect performance (as much) when this feature isn't enabled.
if (!InjectionError.enableBetterErrors) {
return;
}
_tokenStack.removeLast();
}

Expand All @@ -61,12 +56,6 @@ String _noProviderError(Object token) => 'No provider found for $token';
/// builds may be swapped out for less informative errors that cannot be caught;
/// do not rely on being able to catch an [InjectionError] at runtime.
abstract class InjectionError extends AssertionError {
/// May be set to `true` in order to get more debuggable error messages.
///
/// **NOTE**: When assertions are disabled changing this does nothing.
@experimental
static bool enableBetterErrors = true;

InjectionError._();
}

Expand Down

0 comments on commit b9f817e

Please sign in to comment.