fix(ivy): disable use of aliasing in template type-checking (#34649)

FileToModuleHost aliasing supports compilation within environments that have
two properties:

1. A `FileToModuleHost` exists which defines canonical module names for any
   given TS file.
2. Dependency restrictions exist which prevent the import of arbitrary files
   even if such files are within the .d.ts transitive closure of a
   compilation ("strictdeps").

In such an environment, generated imports can only go through import paths
which are already present in the user program. The aliasing system supports
the generation and consumption of such imports at runtime.

`FileToModuleHost` aliasing does not emit re-exports in .d.ts files. This
means that it's safe to rely on alias re-exports in generated .js code (they
are guaranteed to exist at runtime) but not in template type-checking code
(since TS will not be able to follow such imports). Therefore, non-aliased
imports should be used in template type-checking code.

This commit adds a `NoAliasing` flag to `ImportFlags` and sets it when
generating imports in template type-checking code. The testing environment
is also patched to support resolution of FileToModuleHost canonical paths
within the template type-checking program, enabling testing of this change.

PR Close #34649
This commit is contained in:
Alex Rickabaugh 2019-12-19 16:33:56 -08:00 committed by Andrew Kushnir
parent 5b9c96b9b8
commit cb11380515
6 changed files with 109 additions and 3 deletions

View File

@ -209,6 +209,10 @@ export class PrivateExportAliasingHost implements AliasingHost {
*/ */
export class AliasStrategy implements ReferenceEmitStrategy { export class AliasStrategy implements ReferenceEmitStrategy {
emit(ref: Reference<ts.Node>, context: ts.SourceFile, importMode: ImportFlags): Expression|null { emit(ref: Reference<ts.Node>, context: ts.SourceFile, importMode: ImportFlags): Expression|null {
if (importMode & ImportFlags.NoAliasing) {
return null;
}
return ref.alias; return ref.alias;
} }
} }

View File

@ -42,6 +42,14 @@ export enum ImportFlags {
* This is sometimes required if there's a risk TypeScript might remove imports during emit. * This is sometimes required if there's a risk TypeScript might remove imports during emit.
*/ */
ForceNewImport = 0x01, ForceNewImport = 0x01,
/**
* Don't make use of any aliasing information when emitting a reference.
*
* This is sometimes required if emitting into a context where generated references will be fed
* into TypeScript and type-checked (such as in template type-checking).
*/
NoAliasing = 0x02,
} }
/** /**

View File

@ -9,7 +9,7 @@
import {ExpressionType, ExternalExpr, Type, WrappedNodeExpr} from '@angular/compiler'; import {ExpressionType, ExternalExpr, Type, WrappedNodeExpr} from '@angular/compiler';
import * as ts from 'typescript'; import * as ts from 'typescript';
import {NOOP_DEFAULT_IMPORT_RECORDER, Reference, ReferenceEmitter} from '../../imports'; import {ImportFlags, NOOP_DEFAULT_IMPORT_RECORDER, Reference, ReferenceEmitter} from '../../imports';
import {ClassDeclaration, ReflectionHost} from '../../reflection'; import {ClassDeclaration, ReflectionHost} from '../../reflection';
import {ImportManager, translateExpression, translateType} from '../../translator'; import {ImportManager, translateExpression, translateType} from '../../translator';
@ -205,7 +205,11 @@ export class Environment {
* This may involve importing the node into the file if it's not declared there already. * This may involve importing the node into the file if it's not declared there already.
*/ */
reference(ref: Reference<ClassDeclaration<ts.ClassDeclaration>>): ts.Expression { reference(ref: Reference<ClassDeclaration<ts.ClassDeclaration>>): ts.Expression {
const ngExpr = this.refEmitter.emit(ref, this.contextFile); // Disable aliasing for imports generated in a template type-checking context, as there is no
// guarantee that any alias re-exports exist in the .d.ts files. It's safe to use direct imports
// in these cases as there is no strict dependency checking during the template type-checking
// pass.
const ngExpr = this.refEmitter.emit(ref, this.contextFile, ImportFlags.NoAliasing);
// Use `translateExpression` to convert the `Expression` into a `ts.Expression`. // Use `translateExpression` to convert the `Expression` into a `ts.Expression`.
return translateExpression( return translateExpression(
@ -218,7 +222,7 @@ export class Environment {
* This may involve importing the node into the file if it's not declared there already. * This may involve importing the node into the file if it's not declared there already.
*/ */
referenceType(ref: Reference): ts.TypeNode { referenceType(ref: Reference): ts.TypeNode {
const ngExpr = this.refEmitter.emit(ref, this.contextFile); const ngExpr = this.refEmitter.emit(ref, this.contextFile, ImportFlags.NoAliasing);
// Create an `ExpressionType` from the `Expression` and translate it via `translateType`. // Create an `ExpressionType` from the `Expression` and translate it via `translateType`.
// TODO(alxhub): support references to types with generic arguments in a clean way. // TODO(alxhub): support references to types with generic arguments in a clean way.

View File

@ -19,12 +19,18 @@ export class TypeCheckProgramHost implements ts.CompilerHost {
*/ */
private sfMap: Map<string, ts.SourceFile>; private sfMap: Map<string, ts.SourceFile>;
readonly resolveModuleNames?: ts.CompilerHost['resolveModuleNames'];
constructor(sfMap: Map<string, ts.SourceFile>, private delegate: ts.CompilerHost) { constructor(sfMap: Map<string, ts.SourceFile>, private delegate: ts.CompilerHost) {
this.sfMap = sfMap; this.sfMap = sfMap;
if (delegate.getDirectories !== undefined) { if (delegate.getDirectories !== undefined) {
this.getDirectories = (path: string) => delegate.getDirectories !(path); this.getDirectories = (path: string) => delegate.getDirectories !(path);
} }
if (delegate.resolveModuleNames !== undefined) {
this.resolveModuleNames = delegate.resolveModuleNames;
}
} }
getSourceFile( getSourceFile(

View File

@ -248,12 +248,31 @@ class AugmentedCompilerHost extends NgtscCompilerHost {
delegate !: ts.CompilerHost; delegate !: ts.CompilerHost;
} }
const ROOT_PREFIX = 'root/';
class FileNameToModuleNameHost extends AugmentedCompilerHost { class FileNameToModuleNameHost extends AugmentedCompilerHost {
fileNameToModuleName(importedFilePath: string): string { fileNameToModuleName(importedFilePath: string): string {
const relativeFilePath = this.fs.relative(this.fs.pwd(), this.fs.resolve(importedFilePath)); const relativeFilePath = this.fs.relative(this.fs.pwd(), this.fs.resolve(importedFilePath));
const rootedPath = this.fs.join('root', relativeFilePath); const rootedPath = this.fs.join('root', relativeFilePath);
return rootedPath.replace(/(\.d)?.ts$/, ''); return rootedPath.replace(/(\.d)?.ts$/, '');
} }
resolveModuleNames(
moduleNames: string[], containingFile: string, reusedNames: string[]|undefined,
redirectedReference: ts.ResolvedProjectReference|undefined,
options: ts.CompilerOptions): (ts.ResolvedModule|undefined)[] {
return moduleNames.map(moduleName => {
if (moduleName.startsWith(ROOT_PREFIX)) {
// Strip the artificially added root prefix.
moduleName = '/' + moduleName.substr(ROOT_PREFIX.length);
}
return ts
.resolveModuleName(
moduleName, containingFile, options, this, /* cache */ undefined, redirectedReference)
.resolvedModule;
});
}
} }
class MultiCompileHostExt extends AugmentedCompilerHost implements Partial<ts.CompilerHost> { class MultiCompileHostExt extends AugmentedCompilerHost implements Partial<ts.CompilerHost> {

View File

@ -1173,6 +1173,71 @@ export declare class AnimationEvent {
expect(getSourceCodeForDiagnostic(diags[0])).toEqual('y = !y'); expect(getSourceCodeForDiagnostic(diags[0])).toEqual('y = !y');
}); });
it('should still type-check when fileToModuleName aliasing is enabled, but alias exports are not in the .d.ts file',
() => {
// The template type-checking file imports directives/pipes in order to type-check their
// usage. When `FileToModuleHost` aliasing is enabled, these imports would ordinarily use
// aliased values. However, such aliases are not guaranteed to exist in the .d.ts files,
// and so feeding such imports back into TypeScript does not work.
//
// Instead, direct imports should be used within template type-checking code. This test
// verifies that template type-checking is able to cope with such a scenario where
// aliasing is enabled and alias re-exports don't exist in .d.ts files.
env.tsconfig({
// Setting this private flag turns on aliasing.
'_useHostForImportGeneration': true,
// Because the tsconfig is overridden, template type-checking needs to be turned back on
// explicitly as well.
'fullTemplateTypeCheck': true,
});
// 'alpha' declares the directive which will ultimately be imported.
env.write('alpha.d.ts', `
import {ɵɵDirectiveDefWithMeta, ɵɵNgModuleDefWithMeta} from '@angular/core';
export declare class ExternalDir {
input: string;
static ɵdir: ɵɵDirectiveDefWithMeta<ExternalDir, '[test]', never, { 'input': "input" }, never, never>;
}
export declare class AlphaModule {
static ɵmod: ɵɵNgModuleDefWithMeta<AlphaModule, [typeof ExternalDir], never, [typeof ExternalDir]>;
}
`);
// 'beta' re-exports AlphaModule from alpha.
env.write('beta.d.ts', `
import {ɵɵNgModuleDefWithMeta} from '@angular/core';
import {AlphaModule} from './alpha';
export declare class BetaModule {
static ɵmod: ɵɵNgModuleDefWithMeta<BetaModule, never, never, [typeof AlphaModule]>;
}
`);
// The application imports BetaModule from beta, gaining visibility of ExternalDir from
// alpha.
env.write('test.ts', `
import {Component, NgModule} from '@angular/core';
import {BetaModule} from './beta';
@Component({
selector: 'cmp',
template: '<div test input="value"></div>',
})
export class Cmp {}
@NgModule({
declarations: [Cmp],
imports: [BetaModule],
})
export class Module {}
`);
const diags = env.driveDiagnostics();
expect(diags.length).toBe(0);
});
describe('input coercion', () => { describe('input coercion', () => {
beforeEach(() => { beforeEach(() => {
env.tsconfig({fullTemplateTypeCheck: true, strictInputTypes: true}); env.tsconfig({fullTemplateTypeCheck: true, strictInputTypes: true});