Skip to content

Commit

Permalink
fixup! fix(ivy): allow abstract directives to have an invalid constru…
Browse files Browse the repository at this point in the history
…ctor
  • Loading branch information
JoostK committed Oct 7, 2019
1 parent f40a9a9 commit 3ddfa57
Show file tree
Hide file tree
Showing 17 changed files with 137 additions and 67 deletions.
4 changes: 3 additions & 1 deletion packages/compiler-cli/src/ngtsc/annotations/src/component.ts
Expand Up @@ -7,6 +7,7 @@
*/

import {ConstantPool, CssSelector, DEFAULT_INTERPOLATION_CONFIG, DomElementSchemaRegistry, Expression, ExternalExpr, Identifiers, InterpolationConfig, LexerRange, ParseError, ParseSourceFile, ParseTemplateOptions, R3ComponentMetadata, R3TargetBinder, SchemaMetadata, SelectorMatcher, Statement, TmplAstNode, WrappedNodeExpr, compileComponentFromMetadata, makeBindingParser, parseTemplate} from '@angular/compiler';
import {R3FactoryTarget} from '@angular/compiler/src/render3/r3_factory';
import * as ts from 'typescript';

import {CycleAnalyzer} from '../../cycles';
Expand Down Expand Up @@ -487,7 +488,8 @@ export class ComponentDecoratorHandler implements
CompileResult[] {
const meta = analysis.meta;
const res = compileComponentFromMetadata(meta, pool, makeBindingParser());
const factoryRes = compileNgFactoryDefField({...meta, injectFn: Identifiers.directiveInject});
const factoryRes = compileNgFactoryDefField(
{...meta, injectFn: Identifiers.directiveInject, target: R3FactoryTarget.Component});
if (analysis.metadataStmt !== null) {
factoryRes.statements.push(analysis.metadataStmt);
}
Expand Down
6 changes: 4 additions & 2 deletions packages/compiler-cli/src/ngtsc/annotations/src/directive.ts
Expand Up @@ -7,6 +7,7 @@
*/

import {ConstantPool, EMPTY_SOURCE_SPAN, Expression, Identifiers, ParseError, ParsedHostBindings, R3DependencyMetadata, R3DirectiveMetadata, R3QueryMetadata, Statement, WrappedNodeExpr, compileDirectiveFromMetadata, makeBindingParser, parseHostBindings, verifyHostBindings} from '@angular/compiler';
import {R3FactoryTarget} from '@angular/compiler/src/render3/r3_factory';
import * as ts from 'typescript';

import {ErrorCode, FatalDiagnosticError} from '../../diagnostics';
Expand Down Expand Up @@ -94,7 +95,8 @@ export class DirectiveDecoratorHandler implements
CompileResult[] {
const meta = analysis.meta;
const res = compileDirectiveFromMetadata(meta, pool, makeBindingParser());
const factoryRes = compileNgFactoryDefField({...meta, injectFn: Identifiers.directiveInject});
const factoryRes = compileNgFactoryDefField(
{...meta, injectFn: Identifiers.directiveInject, target: R3FactoryTarget.Directive});
if (analysis.metadataStmt !== null) {
factoryRes.statements.push(analysis.metadataStmt);
}
Expand Down Expand Up @@ -234,7 +236,7 @@ export function extractDirectiveMetadata(
}

const rawCtorDeps = getConstructorDependencies(clazz, reflector, defaultImportRecorder, isCore);
let ctorDeps: R3DependencyMetadata[]|'invalid'|null = null;
let ctorDeps: R3DependencyMetadata[]|'invalid'|null;

// Non-abstract directives (those with a selector) require valid constructor dependencies, whereas
// abstract directives are allowed to have invalid dependencies, given that a subclass may call
Expand Down
Expand Up @@ -7,6 +7,7 @@
*/

import {Expression, Identifiers, LiteralExpr, R3DependencyMetadata, R3InjectableMetadata, R3ResolvedDependencyType, Statement, WrappedNodeExpr, compileInjectable as compileIvyInjectable} from '@angular/compiler';
import {R3FactoryTarget} from '@angular/compiler/src/render3/r3_factory';
import * as ts from 'typescript';

import {ErrorCode, FatalDiagnosticError} from '../../diagnostics';
Expand Down Expand Up @@ -83,7 +84,8 @@ export class InjectableDecoratorHandler implements
type: meta.type,
typeArgumentCount: meta.typeArgumentCount,
deps: analysis.ctorDeps,
injectFn: Identifiers.inject
injectFn: Identifiers.inject,
target: R3FactoryTarget.Injectable,
});
if (analysis.metadataStmt !== null) {
factoryRes.statements.push(analysis.metadataStmt);
Expand Down
3 changes: 2 additions & 1 deletion packages/compiler-cli/src/ngtsc/annotations/src/pipe.ts
Expand Up @@ -7,6 +7,7 @@
*/

import {Identifiers, R3PipeMetadata, Statement, WrappedNodeExpr, compilePipeFromMetadata} from '@angular/compiler';
import {R3FactoryTarget} from '@angular/compiler/src/render3/r3_factory';
import * as ts from 'typescript';

import {ErrorCode, FatalDiagnosticError} from '../../diagnostics';
Expand Down Expand Up @@ -112,7 +113,7 @@ export class PipeDecoratorHandler implements DecoratorHandler<PipeHandlerData, D
const factoryRes = compileNgFactoryDefField({
...meta,
injectFn: Identifiers.directiveInject,
isPipe: true,
target: R3FactoryTarget.Pipe,
});
if (analysis.metadataStmt !== null) {
factoryRes.statements.push(analysis.metadataStmt);
Expand Down
6 changes: 6 additions & 0 deletions packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
Expand Up @@ -1375,6 +1375,9 @@ runInEachFileSystem(os => {
const jsContents = env.getContents('test.js');
expect(jsContents)
.toContain('Test.ngFactoryDef = function Test_Factory(t) { throw new Error(');
expect(jsContents)
.toContain(
'("Test has a constructor which is not compatible with Dependency Injection. It should probably not be @Injectable().")');
});

it('should compile an @Injectable provided in the root on a class with a non-injectable constructor',
Expand Down Expand Up @@ -1432,6 +1435,9 @@ runInEachFileSystem(os => {
const jsContents = env.getContents('test.js');
expect(jsContents)
.toContain('Test.ngFactoryDef = function Test_Factory(t) { throw new Error(');
expect(jsContents)
.toContain(
'("Test has a constructor which is not compatible with Dependency Injection.")');
});
});
});
Expand Down
11 changes: 10 additions & 1 deletion packages/compiler/src/compiler_facade_interface.ts
Expand Up @@ -45,6 +45,7 @@ export interface CompilerFacade {
createParseSourceSpan(kind: string, typeName: string, sourceUrl: string): ParseSourceSpan;

R3ResolvedDependencyType: typeof R3ResolvedDependencyType;
R3FactoryTarget: typeof R3FactoryTarget;
ResourceLoader: {new (): ResourceLoader};
}

Expand All @@ -70,6 +71,14 @@ export enum R3ResolvedDependencyType {
ChangeDetectorRef = 2,
}

export enum R3FactoryTarget {
Directive = 0,
Component = 1,
Injectable = 2,
Pipe = 3,
NgModule = 4,
}

export interface R3DependencyMetadataFacade {
token: any;
resolved: R3ResolvedDependencyType;
Expand Down Expand Up @@ -167,7 +176,7 @@ export interface R3FactoryDefMetadataFacade {
typeArgumentCount: number;
deps: R3DependencyMetadataFacade[]|null;
injectFn: 'directiveInject'|'inject';
isPipe: boolean;
target: R3FactoryTarget;
}

export type ViewEncapsulation = number;
Expand Down
52 changes: 30 additions & 22 deletions packages/compiler/src/injectable_compiler_2.ts
Expand Up @@ -8,7 +8,7 @@

import {Identifiers} from './identifiers';
import * as o from './output/output_ast';
import {R3DependencyMetadata, R3FactoryDelegateType, compileFactoryFunction} from './render3/r3_factory';
import {R3DependencyMetadata, R3FactoryDelegateType, R3FactoryTarget, compileFactoryFunction} from './render3/r3_factory';
import {mapToMapExpression, typeWithParameters} from './render3/util';

export interface InjectableDef {
Expand Down Expand Up @@ -56,25 +56,29 @@ export function compileInjectable(meta: R3InjectableMetadata): InjectableDef {

if (deps !== undefined) {
// factory: () => new meta.useClass(...deps)
result = compileFactoryFunction({
...factoryMeta,
delegate: meta.useClass,
delegateDeps: deps,
delegateType: R3FactoryDelegateType.Class,
});
result = compileFactoryFunction(
{
...factoryMeta,
delegate: meta.useClass,
delegateDeps: deps,
delegateType: R3FactoryDelegateType.Class,
},
R3FactoryTarget.Injectable);
} else if (useClassOnSelf) {
result = compileFactoryFunction(factoryMeta);
result = compileFactoryFunction(factoryMeta, R3FactoryTarget.Injectable);
} else {
result = delegateToFactory(meta.useClass);
}
} else if (meta.useFactory !== undefined) {
if (meta.userDeps !== undefined) {
result = compileFactoryFunction({
...factoryMeta,
delegate: meta.useFactory,
delegateDeps: meta.userDeps || [],
delegateType: R3FactoryDelegateType.Function,
});
result = compileFactoryFunction(
{
...factoryMeta,
delegate: meta.useFactory,
delegateDeps: meta.userDeps || [],
delegateType: R3FactoryDelegateType.Function,
},
R3FactoryTarget.Injectable);
} else {
result = {
statements: [],
Expand All @@ -85,16 +89,20 @@ export function compileInjectable(meta: R3InjectableMetadata): InjectableDef {
// Note: it's safe to use `meta.useValue` instead of the `USE_VALUE in meta` check used for
// client code because meta.useValue is an Expression which will be defined even if the actual
// value is undefined.
result = compileFactoryFunction({
...factoryMeta,
expression: meta.useValue,
});
result = compileFactoryFunction(
{
...factoryMeta,
expression: meta.useValue,
},
R3FactoryTarget.Injectable);
} else if (meta.useExisting !== undefined) {
// useExisting is an `inject` call on the existing token.
result = compileFactoryFunction({
...factoryMeta,
expression: o.importExpr(Identifiers.inject).callFn([meta.useExisting]),
});
result = compileFactoryFunction(
{
...factoryMeta,
expression: o.importExpr(Identifiers.inject).callFn([meta.useExisting]),
},
R3FactoryTarget.Injectable);
} else {
result = delegateToFactory(meta.type);
}
Expand Down
5 changes: 3 additions & 2 deletions packages/compiler/src/jit_compiler_facade.ts
Expand Up @@ -16,7 +16,7 @@ import {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from './ml_parser/int
import {DeclareVarStmt, Expression, LiteralExpr, Statement, StmtModifier, WrappedNodeExpr} from './output/output_ast';
import {JitEvaluator} from './output/output_jit';
import {ParseError, ParseSourceSpan, r3JitTypeSourceSpan} from './parse_util';
import {R3DependencyMetadata, R3ResolvedDependencyType, compileFactoryFromMetadata} from './render3/r3_factory';
import {R3DependencyMetadata, R3FactoryTarget, R3ResolvedDependencyType, compileFactoryFromMetadata} from './render3/r3_factory';
import {R3JitReflector} from './render3/r3_jit';
import {R3InjectorMetadata, R3NgModuleMetadata, compileInjector, compileNgModule} from './render3/r3_module_compiler';
import {compilePipeFromMetadata} from './render3/r3_pipe_compiler';
Expand All @@ -29,6 +29,7 @@ import {DomElementSchemaRegistry} from './schema/dom_element_schema_registry';

export class CompilerFacadeImpl implements CompilerFacade {
R3ResolvedDependencyType = R3ResolvedDependencyType as any;
R3FactoryTarget = R3FactoryTarget as any;
ResourceLoader = ResourceLoader;
private elementSchemaRegistry = new DomElementSchemaRegistry();

Expand Down Expand Up @@ -162,7 +163,7 @@ export class CompilerFacadeImpl implements CompilerFacade {
deps: convertR3DependencyMetadataArray(meta.deps),
injectFn: meta.injectFn === 'directiveInject' ? Identifiers.directiveInject :
Identifiers.inject,
isPipe: meta.isPipe
target: meta.target,
});
return this.jitExpression(
factoryRes.factory, angularCoreEnv, sourceMapUrl, factoryRes.statements);
Expand Down
39 changes: 26 additions & 13 deletions packages/compiler/src/render3/r3_factory.ts
Expand Up @@ -82,13 +82,21 @@ export interface R3ExpressionFactoryMetadata extends R3ConstructorFactoryMetadat
export type R3FactoryMetadata = R3ConstructorFactoryMetadata | R3DelegatedFactoryMetadata |
R3DelegatedFnOrClassMetadata | R3ExpressionFactoryMetadata;

export enum R3FactoryTarget {
Directive = 0,
Component = 1,
Injectable = 2,
Pipe = 3,
NgModule = 4,
}

export interface R3FactoryDefMetadata {
name: string;
type: o.Expression;
typeArgumentCount: number;
deps: R3DependencyMetadata[]|'invalid'|null;
injectFn: o.ExternalReference;
isPipe?: boolean;
target: R3FactoryTarget;
}

/**
Expand Down Expand Up @@ -163,7 +171,8 @@ export interface R3FactoryFn {
/**
* Construct a factory function expression for the given `R3FactoryMetadata`.
*/
export function compileFactoryFunction(meta: R3FactoryMetadata, isPipe = false): R3FactoryFn {
export function compileFactoryFunction(
meta: R3FactoryMetadata, target: R3FactoryTarget): R3FactoryFn {
const t = o.variable('t');
const statements: o.Statement[] = [];

Expand All @@ -179,8 +188,9 @@ export function compileFactoryFunction(meta: R3FactoryMetadata, isPipe = false):
if (meta.deps !== null) {
// There is a constructor (either explicitly or implicitly defined).
if (meta.deps !== 'invalid') {
ctorExpr =
new o.InstantiateExpr(typeForCtor, injectDependencies(meta.deps, meta.injectFn, isPipe));
ctorExpr = new o.InstantiateExpr(
typeForCtor,
injectDependencies(meta.deps, meta.injectFn, target === R3FactoryTarget.Pipe));
}
} else {
const baseFactory = o.variable(${meta.name}_BaseFactory`);
Expand All @@ -206,7 +216,7 @@ export function compileFactoryFunction(meta: R3FactoryMetadata, isPipe = false):
if (ctorExprFinal !== null) {
ctorStmt = r.set(ctorExprFinal).toStmt();
} else {
ctorStmt = makeErrorStmt(meta.name);
ctorStmt = makeErrorStmt(meta.name, target);
}
body.push(o.ifStmt(t, [ctorStmt], [r.set(nonCtorExpr).toStmt()]));
return r;
Expand All @@ -228,7 +238,8 @@ export function compileFactoryFunction(meta: R3FactoryMetadata, isPipe = false):
} else if (isDelegatedMetadata(meta)) {
// This type is created with a delegated factory. If a type parameter is not specified, call
// the factory instead.
const delegateArgs = injectDependencies(meta.delegateDeps, meta.injectFn, isPipe);
const delegateArgs =
injectDependencies(meta.delegateDeps, meta.injectFn, target === R3FactoryTarget.Pipe);
// Either call `new delegate(...)` or `delegate(...)` depending on meta.useNewForDelegate.
const factoryExpr = new (
meta.delegateType === R3FactoryDelegateType.Class ?
Expand All @@ -245,7 +256,7 @@ export function compileFactoryFunction(meta: R3FactoryMetadata, isPipe = false):
if (retExpr !== null) {
body.push(new o.ReturnStatement(retExpr));
} else {
body.push(makeErrorStmt(meta.name));
body.push(makeErrorStmt(meta.name, target));
}

return {
Expand All @@ -270,7 +281,7 @@ export function compileFactoryFromMetadata(meta: R3FactoryDefMetadata): R3Factor
typeArgumentCount: meta.typeArgumentCount,
injectFn: meta.injectFn,
},
meta.isPipe);
meta.target);
}

function injectDependencies(
Expand Down Expand Up @@ -358,11 +369,13 @@ export function dependenciesFromGlobalMetadata(
return deps;
}

function makeErrorStmt(name: string): o.Statement {
return new o.ThrowStmt(new o.InstantiateExpr(new o.ReadVarExpr('Error'), [
o.literal(
`${name} has a constructor which is not compatible with Dependency Injection. It should probably not be @Injectable().`)
]));
function makeErrorStmt(name: string, target: R3FactoryTarget): o.Statement {
let message = `${name} has a constructor which is not compatible with Dependency Injection.`;
if (target === R3FactoryTarget.Injectable) {
message += ' It should probably not be @Injectable().';
}

return new o.ThrowStmt(new o.InstantiateExpr(new o.ReadVarExpr('Error'), [o.literal(message)]));
}

function isDelegatedMetadata(meta: R3FactoryMetadata): meta is R3DelegatedFactoryMetadata|
Expand Down
18 changes: 10 additions & 8 deletions packages/compiler/src/render3/r3_module_compiler.ts
Expand Up @@ -12,7 +12,7 @@ import {mapLiteral} from '../output/map_util';
import * as o from '../output/output_ast';
import {OutputContext} from '../util';

import {R3DependencyMetadata, compileFactoryFunction} from './r3_factory';
import {R3DependencyMetadata, R3FactoryTarget, compileFactoryFunction} from './r3_factory';
import {Identifiers as R3} from './r3_identifiers';
import {R3Reference, convertMetaToOutput, mapToMapExpression} from './util';

Expand Down Expand Up @@ -204,13 +204,15 @@ export interface R3InjectorMetadata {
}

export function compileInjector(meta: R3InjectorMetadata): R3InjectorDef {
const result = compileFactoryFunction({
name: meta.name,
type: meta.type,
typeArgumentCount: 0,
deps: meta.deps,
injectFn: R3.inject,
});
const result = compileFactoryFunction(
{
name: meta.name,
type: meta.type,
typeArgumentCount: 0,
deps: meta.deps,
injectFn: R3.inject,
},
R3FactoryTarget.NgModule);
const definitionMap = {
factory: result.factory,
} as{factory: o.Expression, providers: o.Expression, imports: o.Expression};
Expand Down
6 changes: 3 additions & 3 deletions packages/compiler/src/render3/r3_pipe_compiler.ts
Expand Up @@ -12,7 +12,7 @@ import {DefinitionKind} from '../constant_pool';
import * as o from '../output/output_ast';
import {OutputContext, error} from '../util';

import {R3DependencyMetadata, compileFactoryFromMetadata, compileFactoryFunction, dependenciesFromGlobalMetadata} from './r3_factory';
import {R3DependencyMetadata, R3FactoryTarget, compileFactoryFromMetadata, dependenciesFromGlobalMetadata} from './r3_factory';
import {Identifiers as R3} from './r3_identifiers';
import {typeWithParameters} from './util';

Expand Down Expand Up @@ -89,8 +89,8 @@ export function compilePipeFromRender2(
pure: pipe.pure,
};
const res = compilePipeFromMetadata(metadata);
const factoryRes =
compileFactoryFromMetadata({...metadata, injectFn: R3.directiveInject, isPipe: true});
const factoryRes = compileFactoryFromMetadata(
{...metadata, injectFn: R3.directiveInject, target: R3FactoryTarget.Pipe});
const definitionField = outputCtx.constantPool.propertyNameOf(DefinitionKind.Pipe);
const ngFactoryDefStatement = new o.ClassStmt(
/* name */ name,
Expand Down

0 comments on commit 3ddfa57

Please sign in to comment.