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

feat(ivy): implement listing lazy routes for specific entry point in ngtsc #28542

Closed
Closed
38 changes: 34 additions & 4 deletions packages/compiler-cli/src/ngtsc/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {ImportRewriter, ModuleResolver, NoopImportRewriter, R3SymbolsImportRewri
import {PartialEvaluator} from './partial_evaluator';
import {TypeScriptReflectionHost} from './reflection';
import {HostResourceLoader} from './resource_loader';
import {NgModuleRouteAnalyzer} from './routing';
import {NgModuleRouteAnalyzer, entryPointKeyFor} from './routing';
import {FactoryGenerator, FactoryInfo, GeneratedShimsHostWrapper, ShimGenerator, SummaryGenerator, generatedFactoryTransform} from './shims';
import {ivySwitchTransform} from './switch';
import {IvyCompilation, declarationTransformFactory, ivyTransformFactory} from './transform';
Expand Down Expand Up @@ -191,10 +191,40 @@ export class NgtscProgram implements api.Program {
}

listLazyRoutes(entryRoute?: string|undefined): api.LazyRoute[] {
if (entryRoute) {
// Note:
// This resolution step is here to match the implementation of the old `AotCompilerHost` (see
// https://github.com/angular/angular/blob/50732e156/packages/compiler-cli/src/transformers/compiler_host.ts#L175-L188).
//
// `@angular/cli` will always call this API with an absolute path, so the resolution step is
// not necessary, but keeping it backwards compatible in case someone else is using the API.

// Relative entry paths are disallowed.
if (entryRoute.startsWith('.')) {
throw new Error(
`Falied to list lazy routes: Resolution of relative paths (${entryRoute}) is not supported.`);
}

// Non-relative entry paths fall into one of the following categories:
// - Absolute system paths (e.g. `/foo/bar/my-project/my-module`), which are unaffected by the
// logic below.
// - Paths to enternal modules (e.g. `some-lib`).
// - Paths mapped to directories in `tsconfig.json` (e.g. `shared/my-module`).
// (See https://www.typescriptlang.org/docs/handbook/module-resolution.html#path-mapping.)
//
// In all cases above, the `containingFile` argument is ignored, so we can just take the first
// of the root files.
const containingFile = this.tsProgram.getRootFileNames()[0];
const [entryPath, moduleName] = entryRoute.split('#');
const resolved = ts.resolveModuleName(entryPath, containingFile, this.options, this.host);
petebacondarwin marked this conversation as resolved.
Show resolved Hide resolved

if (resolved.resolvedModule) {
entryRoute = entryPointKeyFor(resolved.resolvedModule.resolvedFileName, moduleName);
}
}

this.ensureAnalyzed();
// Listing specific routes is unsupported for now, so we erroneously return
// all lazy routes instead (which should be okay for the CLI's usage).
return this.routeAnalyzer !.listLazyRoutes();
return this.routeAnalyzer !.listLazyRoutes(entryRoute);
}

getLibrarySummaries(): Map<string, api.LibrarySummary> {
Expand Down
1 change: 1 addition & 0 deletions packages/compiler-cli/src/ngtsc/routing/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@
/// <reference types="node" />

export {LazyRoute, NgModuleRouteAnalyzer} from './src/analyzer';
export {entryPointKeyFor} from './src/route';
47 changes: 41 additions & 6 deletions packages/compiler-cli/src/ngtsc/routing/src/analyzer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ import * as ts from 'typescript';
import {ModuleResolver} from '../../imports';
import {PartialEvaluator} from '../../partial_evaluator';

import {scanForRouteEntryPoints} from './lazy';
import {RouterEntryPointManager} from './route';
import {scanForCandidateTransitiveModules, scanForRouteEntryPoints} from './lazy';
import {RouterEntryPointManager, entryPointKeyFor} from './route';

export interface NgModuleRawRouteData {
sourceFile: ts.SourceFile;
Expand All @@ -38,28 +38,63 @@ export class NgModuleRouteAnalyzer {

add(sourceFile: ts.SourceFile, moduleName: string, imports: ts.Expression|null,
exports: ts.Expression|null, providers: ts.Expression|null): void {
const key = `${sourceFile.fileName}#${moduleName}`;
const key = entryPointKeyFor(sourceFile.fileName, moduleName);
if (this.modules.has(key)) {
throw new Error(`Double route analyzing ${key}`);
throw new Error(`Double route analyzing for '${key}'.`);
}
this.modules.set(
key, {
sourceFile, moduleName, imports, exports, providers,
});
}

listLazyRoutes(): LazyRoute[] {
listLazyRoutes(entryModuleKey?: string|undefined): LazyRoute[] {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't it annoying that putting a ? is not enough for the type to be |undefined?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically, they mean different things tho. ? only makes the parameter optional meaning you don't need to set it, but if you do set it then it needs to be string or ...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I know but the end result is the same for the type in the body of the function... if it has a ? then the value might be undefined.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAICT, it doesn't make a different for TS, but I was keeping consistent with program.ts (just in case I miss something).

if ((entryModuleKey !== undefined) && !this.modules.has(entryModuleKey)) {
throw new Error(`Failed to list lazy routes: Unknown module '${entryModuleKey}'.`);
}

const routes: LazyRoute[] = [];
for (const key of Array.from(this.modules.keys())) {
const scannedModuleKeys = new Set<string>();
const pendingModuleKeys = entryModuleKey ? [entryModuleKey] : Array.from(this.modules.keys());

// When listing lazy routes for a specific entry module, we need to recursively extract
// "transitive" routes from imported/exported modules. This is not necessary when listing all
// lazy routes, because all analyzed modules will be scanned anyway.
const scanRecursively = entryModuleKey !== undefined;

while (pendingModuleKeys.length > 0) {
const key = pendingModuleKeys.pop() !;

if (scannedModuleKeys.has(key)) {
continue;
} else {
scannedModuleKeys.add(key);
}

const data = this.modules.get(key) !;
const entryPoints = scanForRouteEntryPoints(
data.sourceFile, data.moduleName, data, this.entryPointManager, this.evaluator);

routes.push(...entryPoints.map(entryPoint => ({
route: entryPoint.loadChildren,
module: entryPoint.from,
referencedModule: entryPoint.resolvedTo,
})));

if (scanRecursively) {
pendingModuleKeys.push(
...[
// Scan the retrieved lazy route entry points.
...entryPoints.map(
({resolvedTo}) => entryPointKeyFor(resolvedTo.filePath, resolvedTo.moduleName)),
// Scan the current module's imported modules.
...scanForCandidateTransitiveModules(data.imports, this.evaluator),
// Scan the current module's exported modules.
...scanForCandidateTransitiveModules(data.exports, this.evaluator),
].filter(key => this.modules.has(key)));
}
}

return routes;
}
}
36 changes: 35 additions & 1 deletion packages/compiler-cli/src/ngtsc/routing/src/lazy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {AbsoluteReference, NodeReference, Reference} from '../../imports';
import {ForeignFunctionResolver, PartialEvaluator, ResolvedValue} from '../../partial_evaluator';

import {NgModuleRawRouteData} from './analyzer';
import {RouterEntryPoint, RouterEntryPointManager} from './route';
import {RouterEntryPoint, RouterEntryPointManager, entryPointKeyFor} from './route';

const ROUTES_MARKER = '__ngRoutesMarker__';

Expand All @@ -22,6 +22,35 @@ export interface LazyRouteEntry {
resolvedTo: RouterEntryPoint;
}

export function scanForCandidateTransitiveModules(
expr: ts.Expression | null, evaluator: PartialEvaluator): string[] {
if (expr === null) {
return [];
}

const candidateModuleKeys: string[] = [];
const entries = evaluator.evaluate(expr);

function recursivelyAddModules(entry: ResolvedValue) {
if (Array.isArray(entry)) {
for (const e of entry) {
recursivelyAddModules(e);
}
} else if (entry instanceof Map) {
if (entry.has('ngModule')) {
recursivelyAddModules(entry.get('ngModule') !);
}
} else if ((entry instanceof Reference) && hasIdentifier(entry.node)) {
const filePath = entry.node.getSourceFile().fileName;
const moduleName = entry.node.name.text;
candidateModuleKeys.push(entryPointKeyFor(filePath, moduleName));
}
}

recursivelyAddModules(entries);
return candidateModuleKeys;
}

export function scanForRouteEntryPoints(
ngModule: ts.SourceFile, moduleName: string, data: NgModuleRawRouteData,
entryPointManager: RouterEntryPointManager, evaluator: PartialEvaluator): LazyRouteEntry[] {
Expand Down Expand Up @@ -152,6 +181,11 @@ const routerModuleFFR: ForeignFunctionResolver =
]);
};

function hasIdentifier(node: ts.Node): node is ts.Node&{name: ts.Identifier} {
const node_ = node as ts.NamedDeclaration;
return (node_.name !== undefined) && ts.isIdentifier(node_.name);
}

function isMethodNodeReference(
ref: Reference<ts.FunctionDeclaration|ts.MethodDeclaration|ts.FunctionExpression>):
ref is NodeReference<ts.MethodDeclaration> {
Expand Down
17 changes: 10 additions & 7 deletions packages/compiler-cli/src/ngtsc/routing/src/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,17 @@ export abstract class RouterEntryPoint {

abstract readonly moduleName: string;

// Alias of moduleName.
// Alias of moduleName for compatibility with what `ngtools_api` returned.
abstract readonly name: string;

abstract toString(): string;
}

class RouterEntryPointImpl implements RouterEntryPoint {
constructor(readonly filePath: string, readonly moduleName: string) {}

get name(): string { return this.moduleName; }

toString(): string { return `${this.filePath}#${this.moduleName}`; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These toString() methods are really helpful when debugging and in errors. I accept that there is no real need for the abstract version since all Objects have this method defined already.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to bring that back then 😃

// For debugging purposes.
toString(): string { return `RouterEntryPoint(name: ${this.name}, filePath: ${this.filePath})`; }
}

export class RouterEntryPointManager {
Expand All @@ -48,11 +47,15 @@ export class RouterEntryPointManager {
}

fromNgModule(sf: ts.SourceFile, moduleName: string): RouterEntryPoint {
const absoluteFile = sf.fileName;
const key = `${absoluteFile}#${moduleName}`;
const key = entryPointKeyFor(sf.fileName, moduleName);
if (!this.map.has(key)) {
this.map.set(key, new RouterEntryPointImpl(absoluteFile, moduleName));
this.map.set(key, new RouterEntryPointImpl(sf.fileName, moduleName));
}
return this.map.get(key) !;
}
}

export function entryPointKeyFor(filePath: string, moduleName: string): string {
// Drop the extension to be compatible with how cli calls `listLazyRoutes(entryRoute)`.
return `${filePath.replace(/\.tsx?$/i, '')}#${moduleName}`;
}
14 changes: 8 additions & 6 deletions packages/compiler-cli/test/ngtools_api_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import {__NGTOOLS_PRIVATE_API_2 as NgTools_InternalApi_NG_2} from '@angular/compiler-cli';
import {fixmeIvy} from '@angular/private/testing';
import {fixmeIvy, ivyEnabled} from '@angular/private/testing';
import * as path from 'path';
import * as ts from 'typescript';

Expand All @@ -19,7 +19,7 @@ describe('ngtools_api (deprecated)', () => {
beforeEach(() => { testSupport = setup(); });

function createProgram(rootNames: string[]) {
const options = testSupport.createCompilerOptions();
const options = testSupport.createCompilerOptions({enableIvy: ivyEnabled && 'ngtsc'});
const host = ts.createCompilerHost(options, true);
const program =
ts.createProgram(rootNames.map(p => path.resolve(testSupport.basePath, p)), options, host);
Expand All @@ -37,7 +37,7 @@ describe('ngtools_api (deprecated)', () => {
export class ErrorComp2 {}

@NgModule({
declarations: [ErrorComp2, NonExistentComp],
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ngtsc complained about non-existent component.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As well it should!

declarations: [ErrorComp2],
imports: [RouterModule.forRoot([{loadChildren: './child#ChildModule'}])]
})
export class MainModule {}
Expand All @@ -60,9 +60,10 @@ describe('ngtools_api (deprecated)', () => {
});
}

fixmeIvy('FW-629: ngtsc lists lazy routes').it('should list lazy routes recursively', () => {
it('should list lazy routes recursively', () => {
writeSomeRoutes();
const {program, host, options} = createProgram(['src/main.ts']);
const {program, host, options} =
createProgram(['src/main.ts', 'src/child.ts', 'src/child2.ts']);
const routes = NgTools_InternalApi_NG_2.listLazyRoutes({
program,
host,
Expand All @@ -77,7 +78,8 @@ describe('ngtools_api (deprecated)', () => {

it('should allow to emit the program after analyzing routes', () => {
writeSomeRoutes();
const {program, host, options} = createProgram(['src/main.ts']);
const {program, host, options} =
createProgram(['src/main.ts', 'src/child.ts', 'src/child2.ts']);
NgTools_InternalApi_NG_2.listLazyRoutes({
program,
host,
Expand Down