Skip to content

Commit

Permalink
feat(compiler-cli): expose function to allow short-circuiting of link…
Browse files Browse the repository at this point in the history
…ing (#40137)

The linker is implemented using a Babel transform such that Babel needs
to parse and walk a source file to find the declarations that need to be
compiled. If it can be determined that a source file is known not to
contain any declarations the parsing and walking can be skipped as a
performance improvement. This commit adds an exposed function for tools
that integrate the linker to use to allow short-circuiting of the linker
transform.

PR Close #40137
  • Loading branch information
JoostK authored and josephperrott committed Dec 22, 2020
1 parent e4fbab9 commit 7dcf286
Show file tree
Hide file tree
Showing 8 changed files with 119 additions and 12 deletions.
1 change: 1 addition & 0 deletions packages/compiler-cli/linker/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,4 @@ export {DeclarationScope} from './src/file_linker/declaration_scope';
export {FileLinker} from './src/file_linker/file_linker';
export {LinkerEnvironment} from './src/file_linker/linker_environment';
export {LinkerOptions} from './src/file_linker/linker_options';
export {needsLinking} from './src/file_linker/needs_linking';
28 changes: 28 additions & 0 deletions packages/compiler-cli/linker/src/file_linker/needs_linking.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/

import {declarationFunctions} from './partial_linkers/partial_linker_selector';

/**
* Determines if the provided source file may need to be processed by the linker, i.e. whether it
* potentially contains any declarations. If true is returned, then the source file should be
* processed by the linker as it may contain declarations that need to be fully compiled. If false
* is returned, parsing and processing of the source file can safely be skipped to improve
* performance.
*
* This function may return true even for source files that don't actually contain any declarations
* that need to be compiled.
*
* @param path the absolute path of the source file for which to determine whether linking may be
* needed.
* @param source the source file content as a string.
* @returns whether the source file may contain declarations that need to be linked.
*/
export function needsLinking(path: string, source: string): boolean {
return declarationFunctions.some(fn => source.includes(fn));
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ import {PartialComponentLinkerVersion1} from './partial_component_linker_1';
import {PartialDirectiveLinkerVersion1} from './partial_directive_linker_1';
import {PartialLinker} from './partial_linker';

export const ɵɵngDeclareDirective = 'ɵɵngDeclareDirective';
export const ɵɵngDeclareComponent = 'ɵɵngDeclareComponent';
export const declarationFunctions = [ɵɵngDeclareDirective, ɵɵngDeclareComponent];

export class PartialLinkerSelector<TExpression> {
/**
* A database of linker instances that should be used if their given semver range satisfies the
Expand All @@ -27,11 +31,11 @@ export class PartialLinkerSelector<TExpression> {
* allows the linker to work on local builds effectively.
*/
private linkers: Record<string, {range: string, linker: PartialLinker<TExpression>}[]> = {
'ɵɵngDeclareDirective': [
[ɵɵngDeclareDirective]: [
{range: '0.0.0-PLACEHOLDER', linker: new PartialDirectiveLinkerVersion1()},
{range: '>=11.1.0-next.1', linker: new PartialDirectiveLinkerVersion1()},
],
'ɵɵngDeclareComponent':
[ɵɵngDeclareComponent]:
[
{range: '0.0.0-PLACEHOLDER', linker: new PartialComponentLinkerVersion1(this.options)},
{range: '>=11.1.0-next.1', linker: new PartialComponentLinkerVersion1(this.options)},
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/

import {needsLinking} from '../../src/file_linker/needs_linking';

describe('needsLinking', () => {
it('should return true for directive declarations', () => {
expect(needsLinking('file.js', `
export class Dir {
ɵdir = ɵɵngDeclareDirective({type: Dir});
}
`)).toBeTrue();
});

it('should return true for namespaced directive declarations', () => {
expect(needsLinking('file.js', `
export class Dir {
ɵdir = ng.ɵɵngDeclareDirective({type: Dir});
}
`)).toBeTrue();
});

it('should return true for unrelated usages of ɵɵngDeclareDirective', () => {
expect(needsLinking('file.js', `
const fnName = 'ɵɵngDeclareDirective';
`)).toBeTrue();
});

it('should return false when the file does not contain ɵɵngDeclareDirective', () => {
expect(needsLinking('file.js', `
const foo = ngDeclareDirective;
`)).toBeFalse();
});

it('should return true for component declarations', () => {
expect(needsLinking('file.js', `
export class Cmp {
ɵdir = ɵɵngDeclareComponent({type: Cmp});
}
`)).toBeTrue();
});

it('should return true for namespaced component declarations', () => {
expect(needsLinking('file.js', `
export class Cmp {
ɵdir = ng.ɵɵngDeclareComponent({type: Cmp});
}
`)).toBeTrue();
});

it('should return true for unrelated usages of ɵɵngDeclareComponent', () => {
expect(needsLinking('file.js', `
const fnName = 'ɵɵngDeclareComponent';
`)).toBeTrue();
});

it('should return false when the file does not contain ɵɵngDeclareComponent', () => {
expect(needsLinking('file.js', `
const foo = ngDeclareComponent;
`)).toBeFalse();
});
});
1 change: 1 addition & 0 deletions packages/compiler-cli/test/compliance/linked/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ ts_library(
testonly = True,
srcs = ["linked_compile_spec.ts"],
deps = [
"//packages/compiler-cli/linker",
"//packages/compiler-cli/linker/babel",
"//packages/compiler-cli/src/ngtsc/file_system",
"//packages/compiler-cli/src/ngtsc/logging",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/
import {PluginObj, transformSync} from '@babel/core';

import {needsLinking} from '../../../linker';
import {createEs2015LinkerPlugin} from '../../../linker/babel';
import {AbsoluteFsPath, FileSystem} from '../../../src/ngtsc/file_system';
import {ConsoleLogger, LogLevel} from '../../../src/ngtsc/logging';
Expand Down Expand Up @@ -57,7 +58,7 @@ function linkPartials(fs: FileSystem, test: ComplianceTest): CompileResult {
const sourceMap =
fs.exists(sourceMapPath) ? JSON.parse(fs.readFile(sourceMapPath)) : undefined;
const {linkedSource, linkedSourceMap} =
applyLinker({fileName, source, sourceMap}, linkerPlugin);
applyLinker({path: partialPath, source, sourceMap}, linkerPlugin);

if (linkedSourceMap !== undefined) {
const mapAndPath: MapAndPath = {map: linkedSourceMap, mapPath: sourceMapPath};
Expand All @@ -75,18 +76,18 @@ function linkPartials(fs: FileSystem, test: ComplianceTest): CompileResult {
*
* It will ignore files that do not have a `.js` extension.
*
* @param file The file name and its source to be transformed using the linker.
* @param file The absolute file path and its source to be transformed using the linker.
* @param linkerPlugin The linker plugin to apply.
* @returns The file's source content, which has been transformed using the linker if necessary.
*/
function applyLinker(
file: {fileName: string; source: string, sourceMap: RawSourceMap | undefined},
file: {path: string; source: string, sourceMap: RawSourceMap | undefined},
linkerPlugin: PluginObj): {linkedSource: string, linkedSourceMap: RawSourceMap|undefined} {
if (!file.fileName.endsWith('.js')) {
if (!file.path.endsWith('.js') || !needsLinking(file.path, file.source)) {
return {linkedSource: file.source, linkedSourceMap: file.sourceMap};
}
const result = transformSync(file.source, {
filename: file.fileName,
filename: file.path,
sourceMaps: !!file.sourceMap,
plugins: [linkerPlugin],
parserOpts: {sourceType: 'unambiguous'},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ ts_library(
srcs = ["bootstrap.ts"],
deps = [
"//packages:types",
"//packages/compiler-cli/linker",
"//packages/compiler-cli/linker/babel",
"//packages/compiler-cli/test/compliance_old/mock_compile",
"@npm//@babel/core",
Expand Down
14 changes: 9 additions & 5 deletions packages/compiler-cli/test/compliance_old/prelink/bootstrap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import {PluginObj, transformSync} from '@babel/core';
import * as ts from 'typescript';

import {needsLinking} from '../../../linker';
import {createEs2015LinkerPlugin} from '../../../linker/babel';
import {compileFiles, CompileFn, setCompileFn} from '../mock_compile';

Expand All @@ -34,24 +35,27 @@ const linkedCompile: CompileFn = (data, angularFiles, options) => {
...options,
});

const source = compiledFiles.map(file => applyLinker(file, linkerPlugin)).join('\n');
const source =
compiledFiles
.map(file => applyLinker({path: file.fileName, source: file.source}, linkerPlugin))
.join('\n');

return {source};
};

/**
* Runs the provided code through the Babel linker plugin, if the file has the .js extension.
*
* @param file The file name and its source to be transformed using the linker.
* @param file The absolute file path and its source to be transformed using the linker.
* @param linkerPlugin The linker plugin to apply.
* @returns The file's source content, which has been transformed using the linker if necessary.
*/
function applyLinker(file: {fileName: string; source: string}, linkerPlugin: PluginObj): string {
if (!file.fileName.endsWith('.js')) {
function applyLinker(file: {path: string; source: string}, linkerPlugin: PluginObj): string {
if (!file.path.endsWith('.js') || !needsLinking(file.path, file.source)) {
return file.source;
}
const result = transformSync(file.source, {
filename: file.fileName,
filename: file.path,
plugins: [linkerPlugin],
parserOpts: {sourceType: 'unambiguous'},
});
Expand Down

0 comments on commit 7dcf286

Please sign in to comment.