Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compile cli fix #8366

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion gulpfile.js
Expand Up @@ -1053,7 +1053,7 @@ gulp.task('!test.compiler_cli.codegen', function(done) {
try {
require('./dist/js/cjs/compiler_cli')
.main("tools/compiler_cli/test")
.then(function() { done(); })
.then(function() { runTsc('tools/compiler_cli/test', done); })
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bug was that compiler_CLI skipped the type check step?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. At least when running the gulp test.compiler_cli target.

.catch(function(rej) { done(new Error(rej)); });
} catch (err) {
done(err);
Expand Down
22 changes: 18 additions & 4 deletions modules/angular2/src/compiler/metadata_resolver.ts
Expand Up @@ -24,7 +24,7 @@ import {LifecycleHooks, LIFECYCLE_HOOKS_VALUES} from 'angular2/src/core/metadata
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, sanitizeIdentifier} from './util';
import {MODULE_SUFFIX, sanitizeIdentifier, ValueTransformer, visitValue} from './util';
import {assertArrayOfStrings} from './assertions';
import {getUrlScheme} from 'angular2/src/compiler/url_resolver';
import {Provider} from 'angular2/src/core/di/provider';
Expand Down Expand Up @@ -314,9 +314,7 @@ export class CompileMetadataResolver {
isPresent(provider.useClass) ?
this.getTypeMetadata(provider.useClass, staticTypeModuleUrl(provider.useClass)) :
null,
useValue: isPresent(provider.useValue) ?
new cpl.CompileIdentifierMetadata({runtime: provider.useValue}) :
null,
useValue: convertToCompileValue(provider.useValue),
useFactory: isPresent(provider.useFactory) ?
this.getFactoryMetadata(provider.useFactory,
staticTypeModuleUrl(provider.useFactory)) :
Expand Down Expand Up @@ -417,3 +415,19 @@ function calcTemplateBaseUrl(reflector: ReflectorReader, type: any,

return reflector.importUri(type);
}

// Only fill CompileIdentifierMetadata.runtime if needed...
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this comment

function convertToCompileValue(value: any): any {
return visitValue(value, new _CompileValueConverter(), null);
}

class _CompileValueConverter extends ValueTransformer {
visitOther(value: any, context: any): any {
if (isStaticType(value)) {
return new cpl.CompileIdentifierMetadata(
{name: value['name'], moduleUrl: staticTypeModuleUrl(value)});
} else {
return new cpl.CompileIdentifierMetadata({runtime: value});
}
}
}
3 changes: 3 additions & 0 deletions modules/angular2/src/compiler/output/dart_emitter.ts
Expand Up @@ -344,6 +344,9 @@ class _DartEmitterVisitor extends AbstractEmitterVisitor implements o.TypeVisito

private _visitIdentifier(value: CompileIdentifierMetadata, typeParams: o.Type[],
ctx: EmitterVisitorContext): void {
if (isBlank(value.name)) {
throw new BaseException(`Internal error: unknown identifier ${value}`);
}
if (isPresent(value.moduleUrl) && value.moduleUrl != this._moduleUrl) {
var prefix = this.importsWithPrefixes.get(value.moduleUrl);
if (isBlank(prefix)) {
Expand Down
4 changes: 4 additions & 0 deletions modules/angular2/src/compiler/output/js_emitter.ts
Expand Up @@ -7,6 +7,7 @@ import {
RegExpWrapper,
StringWrapper
} from 'angular2/src/facade/lang';
import {BaseException} from 'angular2/src/facade/exceptions';
import {OutputEmitter, EmitterVisitorContext} from './abstract_emitter';
import {AbstractJsEmitterVisitor} from './abstract_js_emitter';
import {getImportModulePath, ImportEnv} from './path_util';
Expand Down Expand Up @@ -34,6 +35,9 @@ class JsEmitterVisitor extends AbstractJsEmitterVisitor {
constructor(private _moduleUrl: string) { super(); }

visitExternalExpr(ast: o.ExternalExpr, ctx: EmitterVisitorContext): any {
if (isBlank(ast.value.name)) {
throw new BaseException(`Internal error: unknown identifier ${ast.value}`);
}
if (isPresent(ast.value.moduleUrl) && ast.value.moduleUrl != this._moduleUrl) {
var prefix = this.importsWithPrefixes.get(ast.value.moduleUrl);
if (isBlank(prefix)) {
Expand Down
3 changes: 3 additions & 0 deletions modules/angular2/src/compiler/output/ts_emitter.ts
Expand Up @@ -311,6 +311,9 @@ class _TsEmitterVisitor extends AbstractEmitterVisitor implements o.TypeVisitor

private _visitIdentifier(value: CompileIdentifierMetadata, typeParams: o.Type[],
ctx: EmitterVisitorContext): void {
if (isBlank(value.name)) {
throw new BaseException(`Internal error: unknown identifier ${value}`);
}
if (isPresent(value.moduleUrl) && value.moduleUrl != this._moduleUrl) {
var prefix = this.importsWithPrefixes.get(value.moduleUrl);
if (isBlank(prefix)) {
Expand Down
44 changes: 43 additions & 1 deletion modules/angular2/src/compiler/util.ts
@@ -1,4 +1,13 @@
import {IS_DART, StringWrapper, Math, isBlank} from 'angular2/src/facade/lang';
import {
IS_DART,
StringWrapper,
Math,
isBlank,
isArray,
isStrictStringMap,
isPrimitive
} from 'angular2/src/facade/lang';
import {StringMapWrapper} from 'angular2/src/facade/collection';

export var MODULE_SUFFIX = IS_DART ? '.dart' : '';

Expand Down Expand Up @@ -27,3 +36,36 @@ export function splitAtColon(input: string, defaultValues: string[]): string[] {
export function sanitizeIdentifier(name: string): string {
return StringWrapper.replaceAll(name, /\W/g, '_');
}

export function visitValue(value: any, visitor: ValueVisitor, context: any): any {
if (isArray(value)) {
return visitor.visitArray(<any[]>value, context);
} else if (isStrictStringMap(value)) {
return visitor.visitStringMap(<{[key: string]: any}>value, context);
} else if (isBlank(value) || isPrimitive(value)) {
return visitor.visitPrimitive(value, context);
} else {
return visitor.visitOther(value, context);
}
}

export interface ValueVisitor {
visitArray(arr: any[], context: any): any;
visitStringMap(map: {[key: string]: any}, context: any): any;
visitPrimitive(value: any, context: any): any;
visitOther(value: any, context: any): any;
}

export class ValueTransformer implements ValueVisitor {
visitArray(arr: any[], context: any): any {
return arr.map(value => visitValue(value, this, context));
}
visitStringMap(map: {[key: string]: any}, context: any): any {
var result = {};
StringMapWrapper.forEach(map,
(value, key) => { result[key] = visitValue(value, this, context); });
return result;
}
visitPrimitive(value: any, context: any): any { return value; }
visitOther(value: any, context: any): any { return value; }
}
41 changes: 32 additions & 9 deletions modules/angular2/src/compiler/view_compiler/compile_element.ts
@@ -1,3 +1,4 @@
import {BaseException} from 'angular2/src/facade/exceptions';
import * as o from '../output/output_ast';
import {Identifiers, identifierToken} from '../identifiers';
import {InjectMethodVars} from './constants';
Expand All @@ -18,6 +19,7 @@ import {
import {getPropertyInView, createDiTokenExpression, injectFromViewParentInjector} from './util';
import {CompileQuery, createQueryList, addQueryToTokenMap} from './compile_query';
import {CompileMethod} from './compile_method';
import {ValueTransformer, visitValue} from '../util';

export class CompileNode {
constructor(public parent: CompileElement, public view: CompileView, public nodeIndex: number,
Expand Down Expand Up @@ -72,6 +74,7 @@ export class CompileElement extends CompileNode {
private _createAppElement() {
var fieldName = `_appEl_${this.nodeIndex}`;
var parentNodeIndex = this.isRootElement() ? null : this.parent.nodeIndex;
// private is fine here as no child view will reference an AppElement
this.view.fields.push(new o.ClassField(fieldName, o.importType(Identifiers.AppElement),
[o.StmtModifier.Private]));
var statement = o.THIS_EXPR.prop(fieldName)
Expand Down Expand Up @@ -140,13 +143,7 @@ export class CompileElement extends CompileNode {
return o.importExpr(provider.useClass)
.instantiate(depsExpr, o.importType(provider.useClass));
} else {
if (provider.useValue instanceof CompileIdentifierMetadata) {
return o.importExpr(provider.useValue);
} else if (provider.useValue instanceof o.Expression) {
return provider.useValue;
} else {
return o.literal(provider.useValue);
}
return _convertValueToOutputAst(provider.useValue);
}
});
var propName = `_${resolvedProvider.token.name}_${this.nodeIndex}_${this._instances.size}`;
Expand Down Expand Up @@ -379,11 +376,11 @@ function createProviderProperty(propName: string, provider: ProviderAst,
type = o.DYNAMIC_TYPE;
}
if (isEager) {
view.fields.push(new o.ClassField(propName, type, [o.StmtModifier.Private]));
view.fields.push(new o.ClassField(propName, type));
view.createMethod.addStmt(o.THIS_EXPR.prop(propName).set(resolvedProviderValueExpr).toStmt());
} else {
var internalField = `_${propName}`;
view.fields.push(new o.ClassField(internalField, type, [o.StmtModifier.Private]));
view.fields.push(new o.ClassField(internalField, type));
var getter = new CompileMethod(view);
getter.resetDebugInfo(compileElement.nodeIndex, compileElement.sourceAst);
// Note: Equals is important for JS so that it also checks the undefined case!
Expand All @@ -402,3 +399,29 @@ class _QueryWithRead {
this.read = isPresent(query.meta.read) ? query.meta.read : match;
}
}

function _convertValueToOutputAst(value: any): o.Expression {
return visitValue(value, new _ValueOutputAstTransformer(), null);
}

class _ValueOutputAstTransformer extends ValueTransformer {
visitArray(arr: any[], context: any): o.Expression {
return o.literalArr(arr.map(value => visitValue(value, this, context)));
}
visitStringMap(map: {[key: string]: any}, context: any): o.Expression {
var entries = [];
StringMapWrapper.forEach(
map, (value, key) => { entries.push([key, visitValue(value, this, context)]); });
return o.literalMap(entries);
}
visitPrimitive(value: any, context: any): o.Expression { return o.literal(value); }
visitOther(value: any, context: any): o.Expression {
if (value instanceof CompileIdentifierMetadata) {
return o.importExpr(value);
} else if (value instanceof o.Expression) {
return value;
} else {
throw new BaseException(`Illegal state: Don't now how to compile value ${value}`);
}
}
}
3 changes: 1 addition & 2 deletions modules/angular2/src/compiler/view_compiler/compile_pipe.ts
Expand Up @@ -29,8 +29,7 @@ export class CompilePipe {
}
return injectFromViewParentInjector(diDep.token, false);
});
this.view.fields.push(new o.ClassField(this.instance.name, o.importType(this.meta.type),
[o.StmtModifier.Private]));
this.view.fields.push(new o.ClassField(this.instance.name, o.importType(this.meta.type)));
this.view.createMethod.resetDebugInfo(null, null);
this.view.createMethod.addStmt(o.THIS_EXPR.prop(this.instance.name)
.set(o.importExpr(this.meta.type).instantiate(deps))
Expand Down
3 changes: 1 addition & 2 deletions modules/angular2/src/compiler/view_compiler/compile_query.ts
Expand Up @@ -97,8 +97,7 @@ function mapNestedViews(declarationAppElement: o.Expression, view: CompileView,

export function createQueryList(query: CompileQueryMetadata, directiveInstance: o.Expression,
propertyName: string, compileView: CompileView): o.Expression {
compileView.fields.push(new o.ClassField(propertyName, o.importType(Identifiers.QueryList),
[o.StmtModifier.Private]));
compileView.fields.push(new o.ClassField(propertyName, o.importType(Identifiers.QueryList)));
var expr = o.THIS_EXPR.prop(propertyName);
compileView.createMethod.addStmt(o.THIS_EXPR.prop(propertyName)
.set(o.importExpr(Identifiers.QueryList).instantiate([]))
Expand Down
2 changes: 2 additions & 0 deletions modules/angular2/src/compiler/view_compiler/event_binder.ts
Expand Up @@ -77,6 +77,7 @@ export class CompileEventListener {
(<o.Statement[]>[markPathToRootStart.callMethod('markPathToRootAsCheckOnce', []).toStmt()])
.concat(this._method.finish())
.concat([new o.ReturnStatement(resultExpr)]);
// private is fine here as no child view will reference the event handler...
this.compileElement.view.eventHandlerMethods.push(new o.ClassMethod(
this._methodName, [this._eventParam], stmts, o.BOOL_TYPE, [o.StmtModifier.Private]));
}
Expand All @@ -95,6 +96,7 @@ export class CompileEventListener {
}
var disposable = o.variable(`disposable_${this.compileElement.view.disposables.length}`);
this.compileElement.view.disposables.push(disposable);
// private is fine here as no child view will reference the event handler...
this.compileElement.view.createMethod.addStmt(
disposable.set(listenExpr).toDeclStmt(o.FUNCTION_TYPE, [o.StmtModifier.Private]));
}
Expand Down
Expand Up @@ -43,6 +43,7 @@ function bind(view: CompileView, currValExpr: o.ReadVarExpr, fieldExpr: o.ReadPr
return;
}

// private is fine here as no child view will reference the cached value...
view.fields.push(new o.ClassField(fieldExpr.name, null, [o.StmtModifier.Private]));
view.createMethod.addStmt(
o.THIS_EXPR.prop(fieldExpr.name).set(o.importExpr(Identifiers.uninitialized)).toStmt());
Expand Down
2 changes: 1 addition & 1 deletion modules/angular2/src/compiler/view_compiler/util.ts
Expand Up @@ -88,7 +88,7 @@ export function createFlatArray(expressions: o.Expression[]): o.Expression {

export function createPureProxy(fn: o.Expression, argCount: number, pureProxyProp: o.ReadPropExpr,
view: CompileView) {
view.fields.push(new o.ClassField(pureProxyProp.name, null, [o.StmtModifier.Private]));
view.fields.push(new o.ClassField(pureProxyProp.name, null));
var pureProxyId =
argCount < Identifiers.pureProxies.length ? Identifiers.pureProxies[argCount] : null;
if (isBlank(pureProxyId)) {
Expand Down
11 changes: 4 additions & 7 deletions modules/angular2/src/compiler/view_compiler/view_builder.ts
Expand Up @@ -128,9 +128,8 @@ class ViewBuilderVisitor implements TemplateAstVisitor {
private _visitText(ast: TemplateAst, value: string, ngContentIndex: number,
parent: CompileElement): o.Expression {
var fieldName = `_text_${this.view.nodes.length}`;
this.view.fields.push(new o.ClassField(fieldName,
o.importType(this.view.genConfig.renderTypes.renderText),
[o.StmtModifier.Private]));
this.view.fields.push(
new o.ClassField(fieldName, o.importType(this.view.genConfig.renderTypes.renderText)));
var renderNode = o.THIS_EXPR.prop(fieldName);
var compileNode = new CompileNode(parent, this.view, this.view.nodes.length, renderNode, ast);
var createRenderNode =
Expand Down Expand Up @@ -194,8 +193,7 @@ class ViewBuilderVisitor implements TemplateAstVisitor {
}
var fieldName = `_el_${nodeIndex}`;
this.view.fields.push(
new o.ClassField(fieldName, o.importType(this.view.genConfig.renderTypes.renderElement),
[o.StmtModifier.Private]));
new o.ClassField(fieldName, o.importType(this.view.genConfig.renderTypes.renderElement)));
this.view.createMethod.addStmt(o.THIS_EXPR.prop(fieldName).set(createRenderNodeExpr).toStmt());

var renderNode = o.THIS_EXPR.prop(fieldName);
Expand Down Expand Up @@ -257,8 +255,7 @@ class ViewBuilderVisitor implements TemplateAstVisitor {
var nodeIndex = this.view.nodes.length;
var fieldName = `_anchor_${nodeIndex}`;
this.view.fields.push(
new o.ClassField(fieldName, o.importType(this.view.genConfig.renderTypes.renderComment),
[o.StmtModifier.Private]));
new o.ClassField(fieldName, o.importType(this.view.genConfig.renderTypes.renderComment)));
this.view.createMethod.addStmt(
o.THIS_EXPR.prop(fieldName)
.set(ViewProperties.renderer.callMethod(
Expand Down
1 change: 1 addition & 0 deletions modules/angular2/src/facade/lang.dart
Expand Up @@ -26,6 +26,7 @@ bool isString(Object obj) => obj is String;
bool isFunction(Object obj) => obj is Function;
bool isType(Object obj) => obj is Type;
bool isStringMap(Object obj) => obj is Map;
bool isStrictStringMap(Object obj) => obj is Map;
bool isArray(Object obj) => obj is List;
bool isPromise(Object obj) => obj is Future;
bool isNumber(Object obj) => obj is num;
Expand Down
5 changes: 5 additions & 0 deletions modules/angular2/src/facade/lang.ts
Expand Up @@ -138,6 +138,11 @@ export function isStringMap(obj: any): boolean {
return typeof obj === 'object' && obj !== null;
}

const STRING_MAP_PROTO = Object.getPrototypeOf({});
export function isStrictStringMap(obj: any): boolean {
return isStringMap(obj) && Object.getPrototypeOf(obj) === STRING_MAP_PROTO;
}

export function isPromise(obj: any): boolean {
return obj instanceof (<any>_global).Promise;
}
Expand Down
Expand Up @@ -314,6 +314,27 @@ export function main() {
expect(d.dependency).toBeAnInstanceOf(SimpleDirective);
}));

it('should support useValue with different values', fakeAsync(() => {
var el = createComp('', tcb.overrideProviders(TestComp, [
provide('numLiteral', {useValue: 0}),
provide('boolLiteral', {useValue: true}),
provide('strLiteral', {useValue: 'a'}),
provide('null', {useValue: null}),
provide('array', {useValue: [1]}),
provide('map', {useValue: {'a': 1}}),
provide('instance', {useValue: new TestValue('a')}),
provide('nested', {useValue: [{'a': [1]}, new TestValue('b')]}),
]));
expect(el.inject('numLiteral')).toBe(0);
expect(el.inject('boolLiteral')).toBe(true);
expect(el.inject('strLiteral')).toBe('a');
expect(el.inject('null')).toBe(null);
expect(el.inject('array')).toEqual([1]);
expect(el.inject('map')).toEqual({'a': 1});
expect(el.inject('instance')).toEqual(new TestValue('a'));
expect(el.inject('nested')).toEqual([{'a': [1]}, new TestValue('b')]);
}));

it("should instantiate providers that have dependencies with SkipSelf", fakeAsync(() => {
var el = createComp('<div simpleDirective><span someOtherDirective></span></div>',
tcb.overrideProviders(
Expand Down Expand Up @@ -679,3 +700,7 @@ export function main() {
});
});
}

class TestValue {
constructor(public value: string) {}
}
11 changes: 3 additions & 8 deletions tools/compiler_cli/test/src/basic.ts
@@ -1,14 +1,9 @@
import {Component, Injectable} from 'angular2/core';
import {Component, Inject} from 'angular2/core';
import {FORM_DIRECTIVES} from 'angular2/common';
import {MyComp} from './a/multiple_components';

@Component({
selector: 'basic',
templateUrl: './basic.html',
directives: [MyComp, FORM_DIRECTIVES],
})
@Injectable()
@Component({selector: 'basic', templateUrl: './basic.html', directives: [MyComp, FORM_DIRECTIVES]})
export class Basic {
ctxProp: string;
constructor() { this.ctxProp = 'initial value'; }
constructor() { this.ctxProp = 'initiaValue'; }
}