From 5a3ecedc950a3e2ab5ad75b9bde100cedb11cfe3 Mon Sep 17 00:00:00 2001 From: Brian Ford Date: Wed, 9 Mar 2016 12:05:15 -0800 Subject: [PATCH 1/2] fix(ngFor): give more instructive error when binding to non-iterable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before, you'd get an error like: ``` EXCEPTION: Cannot find a differ supporting object ‘[object Object]’ in [users in UsersCmp@2:14] ``` Now, you get: ``` EXCEPTION: Cannot find a differ supporting object ‘[object Object]’ of type 'Object'. Did you mean to bind ngFor to an Array? in [users in UsersCmp@2:14] ``` --- .../angular2/src/common/directives/ng_for.ts | 10 ++++++++-- .../differs/iterable_differs.ts | 5 +++-- modules/angular2/src/facade/lang.dart | 2 +- modules/angular2/src/facade/lang.ts | 5 ++++- .../test/common/directives/ng_for_spec.ts | 19 +++++++++++++++++++ 5 files changed, 35 insertions(+), 6 deletions(-) diff --git a/modules/angular2/src/common/directives/ng_for.ts b/modules/angular2/src/common/directives/ng_for.ts index b3d315b746c9..4fd502fee32e 100644 --- a/modules/angular2/src/common/directives/ng_for.ts +++ b/modules/angular2/src/common/directives/ng_for.ts @@ -9,11 +9,12 @@ import { EmbeddedViewRef, TrackByFn } from 'angular2/core'; -import {isPresent, isBlank} from 'angular2/src/facade/lang'; +import {isPresent, isBlank, stringify, getTypeNameForDebugging} from 'angular2/src/facade/lang'; import { DefaultIterableDiffer, CollectionChangeRecord } from "../../core/change_detection/differs/default_iterable_differ"; +import {BaseException} from "../../facade/exceptions"; /** * The `NgFor` directive instantiates a template once per item from an iterable. The context for @@ -77,7 +78,12 @@ export class NgFor implements DoCheck { set ngForOf(value: any) { this._ngForOf = value; if (isBlank(this._differ) && isPresent(value)) { - this._differ = this._iterableDiffers.find(value).create(this._cdr, this._ngForTrackBy); + try { + this._differ = this._iterableDiffers.find(value).create(this._cdr, this._ngForTrackBy); + } catch (e) { + throw new BaseException( + `Cannot find a differ supporting object '${value}' of type '${getTypeNameForDebugging(value)}'. NgFor only supports binding to Iterables such as Arrays.`); + } } } diff --git a/modules/angular2/src/core/change_detection/differs/iterable_differs.ts b/modules/angular2/src/core/change_detection/differs/iterable_differs.ts index fd3eb148d33e..8deaf7acc59c 100644 --- a/modules/angular2/src/core/change_detection/differs/iterable_differs.ts +++ b/modules/angular2/src/core/change_detection/differs/iterable_differs.ts @@ -1,4 +1,4 @@ -import {isBlank, isPresent, CONST} from 'angular2/src/facade/lang'; +import {isBlank, isPresent, CONST, getTypeNameForDebugging} from 'angular2/src/facade/lang'; import {BaseException} from 'angular2/src/facade/exceptions'; import {ListWrapper} from 'angular2/src/facade/collection'; import {ChangeDetectorRef} from '../change_detector_ref'; @@ -86,7 +86,8 @@ export class IterableDiffers { if (isPresent(factory)) { return factory; } else { - throw new BaseException(`Cannot find a differ supporting object '${iterable}'`); + throw new BaseException( + `Cannot find a differ supporting object '${iterable}' of type '${getTypeNameForDebugging(iterable)}'`); } } } diff --git a/modules/angular2/src/facade/lang.dart b/modules/angular2/src/facade/lang.dart index 41f7a8dd588f..cafb9fd25731 100644 --- a/modules/angular2/src/facade/lang.dart +++ b/modules/angular2/src/facade/lang.dart @@ -5,7 +5,7 @@ import 'dart:math' as math; import 'dart:convert' as convert; import 'dart:async' show Future, Zone; -String getTypeNameForDebugging(Type type) => type.toString(); +String getTypeNameForDebugging(Object type) => type.toString(); class Math { static final _random = new math.Random(); diff --git a/modules/angular2/src/facade/lang.ts b/modules/angular2/src/facade/lang.ts index edfb50dc5bab..b0d09095756b 100644 --- a/modules/angular2/src/facade/lang.ts +++ b/modules/angular2/src/facade/lang.ts @@ -62,7 +62,10 @@ export interface Type extends Function {} export interface ConcreteType extends Type { new (...args): any; } export function getTypeNameForDebugging(type: Type): string { - return type['name']; + if (type['name']) { + return type['name']; + } + return typeof type; } diff --git a/modules/angular2/test/common/directives/ng_for_spec.ts b/modules/angular2/test/common/directives/ng_for_spec.ts index dd8efbe40f72..12eecb4e2dbd 100644 --- a/modules/angular2/test/common/directives/ng_for_spec.ts +++ b/modules/angular2/test/common/directives/ng_for_spec.ts @@ -14,6 +14,7 @@ import { } from 'angular2/testing_internal'; import {ListWrapper} from 'angular2/src/facade/collection'; +import {IS_DART} from 'angular2/src/facade/lang'; import {Component, TemplateRef, ContentChild} from 'angular2/core'; import {NgFor} from 'angular2/src/common/directives/ng_for'; import {NgIf} from 'angular2/src/common/directives/ng_if'; @@ -158,6 +159,24 @@ export function main() { }); })); + if (!IS_DART) { + it('should throw on non-iterable ref and suggest using an array', + inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => { + tcb.overrideTemplate(TestComponent, TEMPLATE) + .createAsync(TestComponent) + .then((fixture) => { + fixture.debugElement.componentInstance.items = 'whaaa'; + try { + fixture.detectChanges() + } catch (e) { + expect(e.message).toContain( + `Cannot find a differ supporting object 'whaaa' of type 'string'. NgFor only supports binding to Iterables such as Arrays.`); + async.done(); + } + }); + })); + } + it('should throw on ref changing to string', inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => { tcb.overrideTemplate(TestComponent, TEMPLATE) From 9dfa1ccf73f73eff2f327f9f6cb1ce43aed7043f Mon Sep 17 00:00:00 2001 From: Brian Ford Date: Wed, 9 Mar 2016 14:55:27 -0800 Subject: [PATCH 2/2] feat(compiler): assert that Component.style is an array Part of #7481 (effort to improve error messages) --- modules/angular2/src/compiler/assertions.dart | 3 +++ modules/angular2/src/compiler/assertions.ts | 16 ++++++++++++++++ .../angular2/src/compiler/runtime_metadata.ts | 3 +++ .../test/compiler/runtime_metadata_fixture.dart | 9 +++++++++ .../test/compiler/runtime_metadata_fixture.ts | 5 +++++ .../test/compiler/runtime_metadata_spec.ts | 9 +++++++++ 6 files changed, 45 insertions(+) create mode 100644 modules/angular2/src/compiler/assertions.dart create mode 100644 modules/angular2/src/compiler/assertions.ts create mode 100644 modules/angular2/test/compiler/runtime_metadata_fixture.dart create mode 100644 modules/angular2/test/compiler/runtime_metadata_fixture.ts diff --git a/modules/angular2/src/compiler/assertions.dart b/modules/angular2/src/compiler/assertions.dart new file mode 100644 index 000000000000..243292ea57f9 --- /dev/null +++ b/modules/angular2/src/compiler/assertions.dart @@ -0,0 +1,3 @@ +library angular2.core.util.asserions; + +void assertArrayOfStrings(String identifier, Object value) {} diff --git a/modules/angular2/src/compiler/assertions.ts b/modules/angular2/src/compiler/assertions.ts new file mode 100644 index 000000000000..b402485d034a --- /dev/null +++ b/modules/angular2/src/compiler/assertions.ts @@ -0,0 +1,16 @@ +import {isArray, isString, isBlank, assertionsEnabled} from '../facade/lang'; +import {BaseException} from '../facade/exceptions'; + +export function assertArrayOfStrings(identifier: string, value: any) { + if (!assertionsEnabled() || isBlank(value)) { + return; + } + if (!isArray(value)) { + throw new BaseException(`Expected '${identifier}' to be an array of strings.`); + } + for (var i = 0; i < value.length; i += 1) { + if (!isString(value[i])) { + throw new BaseException(`Expected '${identifier}' to be an array of strings.`); + } + } +} diff --git a/modules/angular2/src/compiler/runtime_metadata.ts b/modules/angular2/src/compiler/runtime_metadata.ts index 51b3494b4ef1..d15d2a922909 100644 --- a/modules/angular2/src/compiler/runtime_metadata.ts +++ b/modules/angular2/src/compiler/runtime_metadata.ts @@ -20,6 +20,7 @@ import {reflector} from 'angular2/src/core/reflection/reflection'; import {Injectable, Inject, Optional} from 'angular2/src/core/di'; import {PLATFORM_DIRECTIVES, PLATFORM_PIPES} from 'angular2/src/core/platform_directives_and_pipes'; import {MODULE_SUFFIX} from './util'; +import {assertArrayOfStrings} from './assertions'; import {getUrlScheme} from 'angular2/src/compiler/url_resolver'; @Injectable() @@ -41,9 +42,11 @@ export class RuntimeMetadataResolver { var changeDetectionStrategy = null; if (dirMeta instanceof md.ComponentMetadata) { + assertArrayOfStrings('styles', dirMeta.styles); var cmpMeta = dirMeta; moduleUrl = calcModuleUrl(directiveType, cmpMeta); var viewMeta = this._viewResolver.resolve(directiveType); + assertArrayOfStrings('styles', viewMeta.styles); templateMeta = new cpl.CompileTemplateMetadata({ encapsulation: viewMeta.encapsulation, template: viewMeta.template, diff --git a/modules/angular2/test/compiler/runtime_metadata_fixture.dart b/modules/angular2/test/compiler/runtime_metadata_fixture.dart new file mode 100644 index 000000000000..b26f0f259aae --- /dev/null +++ b/modules/angular2/test/compiler/runtime_metadata_fixture.dart @@ -0,0 +1,9 @@ +library angular2.test.compiler.runtime_metadata_fixture; + +import "package:angular2/core.dart" show Component; + +// This component is not actually malformed; this fixture is here to +// make Dart not complain about a missing import for a test case that only +// matters in an JavaScript app. +@Component(template: "") +class MalformedStylesComponent {} diff --git a/modules/angular2/test/compiler/runtime_metadata_fixture.ts b/modules/angular2/test/compiler/runtime_metadata_fixture.ts new file mode 100644 index 000000000000..74a0a650f3a7 --- /dev/null +++ b/modules/angular2/test/compiler/runtime_metadata_fixture.ts @@ -0,0 +1,5 @@ +import {Component} from 'angular2/core'; + +@Component({styles:('foo'), template: ''}) +export class MalformedStylesComponent { +} diff --git a/modules/angular2/test/compiler/runtime_metadata_spec.ts b/modules/angular2/test/compiler/runtime_metadata_spec.ts index e74a8cae8caf..231d022720e3 100644 --- a/modules/angular2/test/compiler/runtime_metadata_spec.ts +++ b/modules/angular2/test/compiler/runtime_metadata_spec.ts @@ -37,6 +37,7 @@ import {TEST_PROVIDERS} from './test_bindings'; import {MODULE_SUFFIX} from 'angular2/src/compiler/util'; import {IS_DART} from 'angular2/src/facade/lang'; import {PLATFORM_DIRECTIVES} from 'angular2/src/core/platform_directives_and_pipes'; +import {MalformedStylesComponent} from './runtime_metadata_fixture'; export function main() { describe('RuntimeMetadataResolver', () => { @@ -74,6 +75,14 @@ export function main() { var expectedEndValue = IS_DART ? 'test/compiler/runtime_metadata_spec.dart' : './'; expect(value.endsWith(expectedEndValue)).toBe(true); })); + + it('should throw when metadata is incorrectly typed', + inject([RuntimeMetadataResolver], (resolver: RuntimeMetadataResolver) => { + if (!IS_DART) { + expect(() => resolver.getDirectiveMetadata(MalformedStylesComponent)) + .toThrowError(`Expected 'styles' to be an array of strings.`); + } + })); }); describe('getViewDirectivesMetadata', () => {