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

fix(@ngtools/webpack): don't elide imports for type references that … #16822

Merged
merged 1 commit into from
Feb 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 32 additions & 13 deletions packages/ngtools/webpack/src/transformers/ast_helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,14 @@ export function getLastNode(sourceFile: ts.SourceFile): ts.Node | null {
// Test transform helpers.
const basePath = '/project/src/';
const fileName = basePath + 'test-file.ts';
const typeScriptLibFiles = loadTypeScriptLibFiles();
const tsLibFiles = loadTsLibFiles();

export function createTypescriptContext(
content: string,
additionalFiles?: Record<string, string>,
useLibs = false,
importHelpers = true,
extraCompilerOptions: ts.CompilerOptions = {},
) {
// Set compiler options.
const compilerOptions: ts.CompilerOptions = {
Expand All @@ -68,7 +69,9 @@ export function createTypescriptContext(
target: ts.ScriptTarget.ESNext,
skipLibCheck: true,
sourceMap: false,
importHelpers,
importHelpers: true,
experimentalDecorators: true,
...extraCompilerOptions,
};

// Create compiler host.
Expand All @@ -86,11 +89,17 @@ export function createTypescriptContext(
// Write the default libs.
// These are needed for tests that use import(), because it relies on a Promise being there.
const compilerLibFolder = dirname(compilerHost.getDefaultLibFileName(compilerOptions));
for (const [k, v] of Object.entries(tsLibFiles)) {
for (const [k, v] of Object.entries(typeScriptLibFiles)) {
compilerHost.writeFile(join(compilerLibFolder, k), v, false);
}
}

if (compilerOptions.importHelpers) {
for (const [k, v] of Object.entries(tsLibFiles)) {
compilerHost.writeFile(k, v, false);
}
}

if (additionalFiles) {
for (const key in additionalFiles) {
compilerHost.writeFile(basePath + key, additionalFiles[key], false);
Expand All @@ -108,8 +117,7 @@ export function transformTypescript(
transformers: ts.TransformerFactory<ts.SourceFile>[],
program?: ts.Program,
compilerHost?: WebpackCompilerHost,
) {

): string | undefined {
// Use given context or create a new one.
if (content !== undefined) {
const typescriptContext = createTypescriptContext(content);
Expand Down Expand Up @@ -137,18 +145,29 @@ export function transformTypescript(
return compilerHost.readFile(fileName.replace(/\.tsx?$/, '.js'));
}

function loadTsLibFiles() {
function loadTypeScriptLibFiles(): Record<string, string> {
const libFolderPath = dirname(require.resolve('typescript/lib/lib.d.ts'));
const libFolderFiles = readdirSync(libFolderPath);
const libFileNames = libFolderFiles.filter(f => f.startsWith('lib.') && f.endsWith('.d.ts'));

// Return a map of the lib names to their content.
return libFileNames.reduce(
(map, f) => {
map[f] = readFileSync(join(libFolderPath, f), 'utf-8');
const libs: Record<string, string> = {};
for (const f of libFileNames) {
libs[f] = readFileSync(join(libFolderPath, f), 'utf-8');
}

return libs;
}

function loadTsLibFiles(): Record<string, string> {
const libFolderPath = dirname(require.resolve('tslib/package.json'));
const libFolderFiles = readdirSync(libFolderPath);

// Return a map of the lib names to their content.
const libs: Record<string, string> = {};
for (const f of libFolderFiles) {
libs[join('node_modules/tslib', f)] = readFileSync(join(libFolderPath, f), 'utf-8');
}

return map;
},
{} as { [k: string]: string },
);
return libs;
}
54 changes: 43 additions & 11 deletions packages/ngtools/webpack/src/transformers/elide_imports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export function elideImports(
sourceFile: ts.SourceFile,
removedNodes: ts.Node[],
getTypeChecker: () => ts.TypeChecker,
compilerOptions: ts.CompilerOptions,
): TransformOperation[] {
const ops: TransformOperation[] = [];

Expand All @@ -33,8 +34,8 @@ export function elideImports(
const imports: ts.ImportDeclaration[] = [];

ts.forEachChild(sourceFile, function visit(node) {
// Skip type references and removed nodes. We consider both unused.
if (node.kind == ts.SyntaxKind.TypeReference || removedNodes.includes(node)) {
// Skip removed nodes.
if (removedNodes.includes(node)) {
return;
}

Expand All @@ -46,17 +47,48 @@ export function elideImports(
}

let symbol: ts.Symbol | undefined;
if (ts.isTypeReferenceNode(node)) {
if (!compilerOptions.emitDecoratorMetadata) {
// Skip and mark as unused if emitDecoratorMetadata is disabled.
return;
}

switch (node.kind) {
case ts.SyntaxKind.Identifier:
const parent = node.parent;
let isTypeReferenceForDecoratoredNode = false;

switch (parent.kind) {
case ts.SyntaxKind.GetAccessor:
case ts.SyntaxKind.PropertyDeclaration:
case ts.SyntaxKind.MethodDeclaration:
isTypeReferenceForDecoratoredNode = !!parent.decorators?.length;
break;
case ts.SyntaxKind.Parameter:
// - A constructor parameter can be decorated or the class itself is decorated.
// - The parent of the parameter is decorated example a method declaration or a set accessor.
// In all cases we need the type reference not to be elided.
isTypeReferenceForDecoratoredNode = !!(parent.decorators?.length ||
(ts.isSetAccessor(parent.parent) && !!parent.parent.decorators?.length) ||
(ts.isConstructorDeclaration(parent.parent) && !!parent.parent.parent.decorators?.length));
break;
}
if (isTypeReferenceForDecoratoredNode) {
symbol = typeChecker.getSymbolAtLocation(node);
break;
case ts.SyntaxKind.ExportSpecifier:
symbol = typeChecker.getExportSpecifierLocalTargetSymbol(node as ts.ExportSpecifier);
break;
case ts.SyntaxKind.ShorthandPropertyAssignment:
symbol = typeChecker.getShorthandAssignmentValueSymbol(node);
break;
} else {
// If type reference is not for Decorator skip and marked as unused.
return;
}
} else {
switch (node.kind) {
case ts.SyntaxKind.Identifier:
symbol = typeChecker.getSymbolAtLocation(node);
break;
case ts.SyntaxKind.ExportSpecifier:
symbol = typeChecker.getExportSpecifierLocalTargetSymbol(node as ts.ExportSpecifier);
break;
case ts.SyntaxKind.ShorthandPropertyAssignment:
symbol = typeChecker.getShorthandAssignmentValueSymbol(node);
break;
}
}

if (symbol) {
Expand Down
Loading