iSharkFly-Docs
/
angular-docs-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(compiler-cli): set TS original node on imported namespace identifiers (#40711) 

This commit causes imports added by ngtsc's `ImportManager` to have their
TypeScript "original node" set to the generated `ts.ImportDeclaration`
statement.

In g3, the tsickle transformer runs after the Angular transformer and post-
processes Angular's compilation output. One of its post-processing tasks is
to transform generated imports and references to imported symbols from the
commonjs module system to the g3 module system. Part of this transformation
involves recognizing modules with specific metadata and altering references
to symbols from those modules accordingly.

Normally, tsickle can rely on TypeScript's binding for an imported symbol to
find its origin module and thus the correct metadata for the symbol. However
the Angular transform generates new synthetic imports which don't have such
binding information. Angular's imports are always namespace imports of the
form:

```
import * as qualifier 'module/specifier';
```

References to such an import are then of the form `qualifier.SymbolName`.

To process such imports properly, tsickle needs to be able to associate the
reference to `qualifier` in the expression `qualifer.SymbolName` with the
`ts.ImportDeclaration` statement that defines it. It expects to do this by
looking at the `ts.getOriginalNode()` for the `qualifier` reference, which
should be the `ts.ImportDeclaration`. This commit changes ngtsc's import
generation mechanism to set the original node on `qualifier` identifiers
according to this expectation.

This commit is not tested in the direct compiler tests, since:

1) there is no observable behavior externally from setting the original node
2) we don't have tests that intercept transformer operations (which could be
   used to directly assert against the AST nodes)
3) tsickle's published version does not (yet) contain the g3-specific
   transformations which rely on the original node and would thus allow the
   behavior to be observed.

Instead, we rely on the g3 testing suite to validate the correctness of this
fix. Breaking this functionality would cause g3 compilation errors for
targets, since tsickle would be unable to transform imports correctly.

PR Close #40711
This commit is contained in:
Alex Rickabaugh 2021-02-08 13:32:04 -08:00 committed by Joey Perrott
parent 5eb195416b
commit 80f4ff3338
14 changed files with 74 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
packages/compiler-cli/ngcc/src/rendering/commonjs_rendering_formatter.ts
View File

@ -39,7 +39,7 @@ export class CommonJsRenderingFormatter extends Esm5RenderingFormatter {
const insertionPoint = this.findEndOfImports(file);
const renderedImports =
imports.map(i => `var ${i.qualifier} = require('${i.specifier}');\n`).join('');
imports.map(i => `var ${i.qualifier.text} = require('${i.specifier}');\n`).join('');
output.appendLeft(insertionPoint, renderedImports);
}

2
packages/compiler-cli/ngcc/src/rendering/esm_rendering_formatter.ts
View File

@ -42,7 +42,7 @@ export class EsmRenderingFormatter implements RenderingFormatter {
const insertionPoint = this.findEndOfImports(sf);
const renderedImports =
imports.map(i => `import * as ${i.qualifier} from '${i.specifier}';\n`).join('');
imports.map(i => `import * as ${i.qualifier.text} from '${i.specifier}';\n`).join('');
output.appendLeft(insertionPoint, renderedImports);
}

2
packages/compiler-cli/ngcc/src/rendering/umd_rendering_formatter.ts
View File

@ -225,7 +225,7 @@ function renderFactoryParameters(
}
const parameters = factoryFunction.parameters;
const parameterString = imports.map(i => i.qualifier).join(',');
const parameterString = imports.map(i => i.qualifier.text).join(',');
if (parameters.length > 0) {
const injectionPoint = parameters[0].getFullStart();
output.appendLeft(injectionPoint, parameterString + ',');

7
packages/compiler-cli/ngcc/test/rendering/commonjs_rendering_formatter_spec.ts
View File

@ -177,8 +177,8 @@ exports.D = D;
renderer.addImports(
output,
[
{specifier: '@angular/core', qualifier: 'i0'},
{specifier: '@angular/common', qualifier: 'i1'}
{specifier: '@angular/core', qualifier: ts.createIdentifier('i0')},
{specifier: '@angular/common', qualifier: ts.createIdentifier('i1')}
],
sourceFile);
expect(output.toString()).toContain(`/* A copyright notice */
@ -257,7 +257,8 @@ var A = (function() {`);
const file = getSourceFileOrError(program, _('/node_modules/test-package/some/file.js'));
const output = new MagicString(PROGRAM.contents);
renderer.addConstants(output, 'var x = 3;', file);
renderer.addImports(output, [{specifier: '@angular/core', qualifier: 'i0'}], file);
renderer.addImports(
output, [{specifier: '@angular/core', qualifier: ts.createIdentifier('i0')}], file);
expect(output.toString()).toContain(`
var core = require('@angular/core');
var i0 = require('@angular/core');

7
packages/compiler-cli/ngcc/test/rendering/esm5_rendering_formatter_spec.ts
View File

@ -188,8 +188,8 @@ export { F };
renderer.addImports(
output,
[
{specifier: '@angular/core', qualifier: 'i0'},
{specifier: '@angular/common', qualifier: 'i1'}
{specifier: '@angular/core', qualifier: ts.createIdentifier('i0')},
{specifier: '@angular/common', qualifier: ts.createIdentifier('i1')}
],
sourceFile);
expect(output.toString()).toContain(`/* A copyright notice */
@ -263,7 +263,8 @@ var A = (function() {`);
const file = getSourceFileOrError(program, _('/node_modules/test-package/some/file.js'));
const output = new MagicString(PROGRAM.contents);
renderer.addConstants(output, 'var x = 3;', file);
renderer.addImports(output, [{specifier: '@angular/core', qualifier: 'i0'}], file);
renderer.addImports(
output, [{specifier: '@angular/core', qualifier: ts.createIdentifier('i0')}], file);
expect(output.toString()).toContain(`
import {Directive} from '@angular/core';
import * as i0 from '@angular/core';

7
packages/compiler-cli/ngcc/test/rendering/esm_rendering_formatter_spec.ts
View File

@ -156,8 +156,8 @@ runInEachFileSystem(() => {
renderer.addImports(
output,
[
{specifier: '@angular/core', qualifier: 'i0'},
{specifier: '@angular/common', qualifier: 'i1'}
{specifier: '@angular/core', qualifier: ts.createIdentifier('i0')},
{specifier: '@angular/common', qualifier: ts.createIdentifier('i1')}
],
sourceFile);
expect(output.toString()).toContain(`/* A copyright notice */
@ -231,7 +231,8 @@ const x = 3;
const file = getSourceFileOrError(program, _('/node_modules/test-package/some/file.js'));
const output = new MagicString(PROGRAM.contents);
renderer.addConstants(output, 'const x = 3;', file);
renderer.addImports(output, [{specifier: '@angular/core', qualifier: 'i0'}], file);
renderer.addImports(
output, [{specifier: '@angular/core', qualifier: ts.createIdentifier('i0')}], file);
expect(output.toString()).toContain(`
import {Directive} from '@angular/core';
import * as i0 from '@angular/core';

4
packages/compiler-cli/ngcc/test/rendering/renderer_spec.ts
View File

@ -276,7 +276,7 @@ A.ɵcmp = ɵngcc0.ɵɵdefineComponent({ type: A, selectors: [["a"]], decls: 1, v
const addImportsSpy = testFormatter.addImports as jasmine.Spy;
expect(addImportsSpy.calls.first().args[0].toString()).toEqual(RENDERED_CONTENTS);
expect(addImportsSpy.calls.first().args[1]).toEqual([
{specifier: '@angular/core', qualifier: 'ɵngcc0'}
{specifier: '@angular/core', qualifier: jasmine.objectContaining({text: 'ɵngcc0'})}
]);
});
@ -693,7 +693,7 @@ UndecoratedBase.ɵdir = ɵngcc0.ɵɵdefineDirective({ type: UndecoratedBase, vie
`function () { (typeof ngDevMode === "undefined" || ngDevMode) && ɵngcc0.setClassMetadata(`);
const addImportsSpy = testFormatter.addImports as jasmine.Spy;
expect(addImportsSpy.calls.first().args[1]).toEqual([
{specifier: './r3_symbols', qualifier: 'ɵngcc0'}
{specifier: './r3_symbols', qualifier: jasmine.objectContaining({text: 'ɵngcc0'})}
]);
});

38
packages/compiler-cli/ngcc/test/rendering/umd_rendering_formatter_spec.ts
View File

@ -200,8 +200,8 @@ typeof define === 'function' && define.amd ? define('file', ['exports','/tslib',
renderer.addImports(
output,
[
{specifier: '@angular/core', qualifier: 'i0'},
{specifier: '@angular/common', qualifier: 'i1'}
{specifier: '@angular/core', qualifier: ts.createIdentifier('i0')},
{specifier: '@angular/common', qualifier: ts.createIdentifier('i1')}
],
file);
expect(output.toString())
@ -217,8 +217,8 @@ typeof define === 'function' && define.amd ? define('file', ['exports','/tslib',
renderer.addImports(
output,
[
{specifier: '@angular/core', qualifier: 'i0'},
{specifier: '@angular/common', qualifier: 'i1'}
{specifier: '@angular/core', qualifier: ts.createIdentifier('i0')},
{specifier: '@angular/common', qualifier: ts.createIdentifier('i1')}
],
file);
expect(output.toString())
@ -233,8 +233,8 @@ typeof define === 'function' && define.amd ? define('file', ['exports','/tslib',
renderer.addImports(
output,
[
{specifier: '@angular/core', qualifier: 'i0'},
{specifier: '@angular/common', qualifier: 'i1'}
{specifier: '@angular/core', qualifier: ts.createIdentifier('i0')},
{specifier: '@angular/common', qualifier: ts.createIdentifier('i1')}
],
file);
expect(output.toString())
@ -249,10 +249,12 @@ typeof define === 'function' && define.amd ? define('file', ['exports','/tslib',
renderer.addImports(
output,
[
{specifier: '@ngrx/store', qualifier: 'i0'},
{specifier: '@angular/platform-browser-dynamic', qualifier: 'i1'},
{specifier: '@angular/common/testing', qualifier: 'i2'},
{specifier: '@angular-foo/package', qualifier: 'i3'}
{specifier: '@ngrx/store', qualifier: ts.createIdentifier('i0')}, {
specifier: '@angular/platform-browser-dynamic',
qualifier: ts.createIdentifier('i1')
},
{specifier: '@angular/common/testing', qualifier: ts.createIdentifier('i2')},
{specifier: '@angular-foo/package', qualifier: ts.createIdentifier('i3')}
],
file);
expect(output.toString())
@ -270,8 +272,8 @@ typeof define === 'function' && define.amd ? define('file', ['exports','/tslib',
renderer.addImports(
output,
[
{specifier: '@angular/core', qualifier: 'i0'},
{specifier: '@angular/common', qualifier: 'i1'}
{specifier: '@angular/core', qualifier: ts.createIdentifier('i0')},
{specifier: '@angular/common', qualifier: ts.createIdentifier('i1')}
],
file);
expect(output.toString())
@ -287,8 +289,8 @@ typeof define === 'function' && define.amd ? define('file', ['exports','/tslib',
renderer.addImports(
output,
[
{specifier: '@angular/core', qualifier: 'i0'},
{specifier: '@angular/common', qualifier: 'i1'}
{specifier: '@angular/core', qualifier: ts.createIdentifier('i0')},
{specifier: '@angular/common', qualifier: ts.createIdentifier('i1')}
],
file);
expect(output.toString())
@ -315,8 +317,8 @@ typeof define === 'function' && define.amd ? define('file', ['exports','/tslib',
renderer.addImports(
output,
[
{specifier: '@angular/core', qualifier: 'i0'},
{specifier: '@angular/common', qualifier: 'i1'}
{specifier: '@angular/core', qualifier: ts.createIdentifier('i0')},
{specifier: '@angular/common', qualifier: ts.createIdentifier('i1')}
],
file);
const outputSrc = output.toString();
@ -361,8 +363,8 @@ typeof define === 'function' && define.amd ? define('file', ['exports','/tslib',
renderer.addImports(
output,
[
{specifier: '@angular/core', qualifier: 'i0'},
{specifier: '@angular/common', qualifier: 'i1'}
{specifier: '@angular/core', qualifier: ts.createIdentifier('i0')},
{specifier: '@angular/common', qualifier: ts.createIdentifier('i1')}
],
file);
const outputSrc = output.toString();

16
packages/compiler-cli/src/ngtsc/transform/src/utils.ts
View File

@ -19,15 +19,27 @@ export function addImports(
extraStatements: ts.Statement[] = []): ts.SourceFile {
// Generate the import statements to prepend.
const addedImports = importManager.getAllImports(sf.fileName).map(i => {
const qualifier = ts.createIdentifier(i.qualifier);
const qualifier = ts.createIdentifier(i.qualifier.text);
const importClause = ts.createImportClause(
/* name */ undefined,
/* namedBindings */ ts.createNamespaceImport(qualifier));
return ts.createImportDeclaration(
const decl = ts.createImportDeclaration(
/* decorators */ undefined,
/* modifiers */ undefined,
/* importClause */ importClause,
/* moduleSpecifier */ ts.createLiteral(i.specifier));
// Set the qualifier's original TS node to the `ts.ImportDeclaration`. This allows downstream
// transforms such as tsickle to properly process references to this import.
//
// This operation is load-bearing in g3 as some imported modules contain special metadata
// generated by clutz, which tsickle uses to transform imports and references to those imports.
//
// TODO(alxhub): add a test for this when tsickle is updated externally to depend on this
// behavior.
ts.setOriginalNode(i.qualifier, decl);
return decl;
});
// Filter out the existing imports and the source file body. All new statements

4
packages/compiler-cli/src/ngtsc/translator/index.ts
View File

@ -7,9 +7,9 @@
*/
export {AstFactory, BinaryOperator, LeadingComment, ObjectLiteralProperty, SourceMapLocation, SourceMapRange, TemplateElement, TemplateLiteral, UnaryOperator, VariableDeclarationType} from './src/api/ast_factory';
export {Import, ImportGenerator, NamedImport} from './src/api/import_generator';
export {ImportGenerator, NamedImport} from './src/api/import_generator';
export {Context} from './src/context';
export {ImportManager} from './src/import_manager';
export {Import, ImportManager} from './src/import_manager';
export {ExpressionTranslatorVisitor, RecordWrappedNodeExprFn, TranslatorOptions} from './src/translator';
export {translateType} from './src/type_translator';
export {attachComments, createTemplateMiddle, createTemplateTail, TypeScriptAstFactory} from './src/typescript_ast_factory';

10
packages/compiler-cli/src/ngtsc/translator/src/api/import_generator.ts
View File

@ -6,16 +6,6 @@
* found in the LICENSE file at https://angular.io/license
*/
/**
* Information about an import that has been added to a module.
*/
export interface Import {
/** The name of the module that has been imported. */
specifier: string;
/** The alias of the imported module. */
qualifier: string;
}
/**
* The symbol name and import namespace of an imported symbol,
* which has been registered through the ImportGenerator.

25
packages/compiler-cli/src/ngtsc/translator/src/import_manager.ts
View File

@ -7,7 +7,17 @@
*/
import * as ts from 'typescript';
import {ImportRewriter, NoopImportRewriter} from '../../imports';
import {Import, ImportGenerator, NamedImport} from './api/import_generator';
import {ImportGenerator, NamedImport} from './api/import_generator';
/**
* Information about an import that has been added to a module.
*/
export interface Import {
/** The name of the module that has been imported. */
specifier: string;
/** The `ts.Identifer` by which the imported module is known. */
qualifier: ts.Identifier;
}
export class ImportManager implements ImportGenerator<ts.Identifier> {
private specifierToIdentifier = new Map<string, ts.Identifier>();
@ -42,11 +52,14 @@ export class ImportManager implements ImportGenerator<ts.Identifier> {
}
getAllImports(contextPath: string): Import[] {
const imports: {specifier: string, qualifier: string}[] = [];
this.specifierToIdentifier.forEach((qualifier, specifier) => {
specifier = this.rewriter.rewriteSpecifier(specifier, contextPath);
imports.push({specifier, qualifier: qualifier.text});
});
const imports: Import[] = [];
for (const [originalSpecifier, qualifier] of this.specifierToIdentifier) {
const specifier = this.rewriter.rewriteSpecifier(originalSpecifier, contextPath);
imports.push({
specifier,
qualifier,
});
}
return imports;
}
}

2
packages/compiler-cli/src/ngtsc/typecheck/src/context.ts
View File

@ -355,7 +355,7 @@ export class TypeCheckContextImpl implements TypeCheckContext {
// Write out the imports that need to be added to the beginning of the file.
let imports = importManager.getAllImports(sf.fileName)
.map(i => `import * as ${i.qualifier} from '${i.specifier}';`)
.map(i => `import * as ${i.qualifier.text} from '${i.specifier}';`)
.join('\n');
code = imports + '\n' + code;

2
packages/compiler-cli/src/ngtsc/typecheck/src/type_check_file.ts
View File

@ -51,7 +51,7 @@ export class TypeCheckFile extends Environment {
render(): string {
let source: string = this.importManager.getAllImports(this.contextFile.fileName)
.map(i => `import * as ${i.qualifier} from '${i.specifier}';`)
.map(i => `import * as ${i.qualifier.text} from '${i.specifier}';`)
.join('\n') +
'\n\n';
const printer = ts.createPrinter();