Skip to content
This repository has been archived by the owner on Mar 6, 2024. It is now read-only.

Commit

Permalink
Fix route-analyzer not handling equal module basenames
Browse files Browse the repository at this point in the history
Before this commit, the route-analyzer would search for modules by their basenames (my-module.ts). This could match the wrong module in cases where we had two module files with same name but in different folders.

This change fixes the issue by matching absolute paths instead of only basenames.

Signed-off-by: Davi Barreto <[email protected]>
  • Loading branch information
Davi Barreto authored and juanmendes committed Jan 18, 2023
1 parent 22af030 commit 15fd4cb
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 7 deletions.
2 changes: 2 additions & 0 deletions projects/route-analyzer/CHANGELOG.MD
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]
### [Fixed]
- Fixed bug that broke the route-analyzer when two routing modules had the same base name

## [0.0.6]
### [Added]
Expand Down
31 changes: 24 additions & 7 deletions projects/route-analyzer/src/lib/route-analyzer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* Copyright 2020 VMware, Inc.
* SPDX-License-Identifier: BSD-2-Clause
*/
import * as path from 'path';
import * as ts from 'typescript';
import { AppRoute } from './app-route';
import { ANGULAR_CORE, ROUTE_PROP } from './constants';
Expand Down Expand Up @@ -54,7 +55,11 @@ export function getRoutes(program: ts.Program): AppRoute[] {
.filter(hasValue) // Not all the files that contain router usage, contain router configuration call
.map((moduleRouteCall) => routeCallToRoutes(moduleRouteCall, program.getTypeChecker()));

const moduleAppRoutes: ModuleAppRoutes[] = fixupLoadChildren(moduleRoutes, moduleFiles);
const moduleAppRoutes: ModuleAppRoutes[] = fixupLoadChildren(
moduleRoutes,
moduleFiles,
program.getCurrentDirectory()
);

// Get only the routes into a single array
const appRoutes: AppRoute[] = moduleAppRoutes
Expand Down Expand Up @@ -228,7 +233,8 @@ function getModulefromLoadChildrenCall(expr: ts.Expression): string {
*/
function fixupLoadChildren(
moduleRoutesWithLoadChildren: ModuleRoutes[],
allModuleFiles: ts.SourceFile[]
allModuleFiles: ts.SourceFile[],
programDirectory: string
): ModuleAppRoutes[] {
// Used for optimization: instead of an array, build a map
const routesByModule = new Map(
Expand All @@ -240,12 +246,13 @@ function fixupLoadChildren(

// Lazy loaded module references are in fact substituted with the actual route configuration
const moduleRoutes: ModuleRoutes[] = moduleRoutesWithLoadChildren.map((moduleRoute) => {
const moduleFolderPath = path.dirname(moduleRoute.module.fileName);
const routes = moduleRoute.routes.map((route) => {
if (!route.loadChildren) {
return route;
}
const { loadChildren, ...routeWithNoLoadChildren } = route;
const referencedModule = getReferencedModule(loadChildren);
const referencedModule = getReferencedModule(getLoadChildrenAbsolutePath(moduleFolderPath, loadChildren));
const children: AppRoute[] = [...routesByModule.get(referencedModule)];
referencedLazyLoadedModules[referencedModule.fileName] = true;
const newRoute: AppRoute = {
Expand All @@ -261,16 +268,15 @@ function fixupLoadChildren(
return moduleRoutes.filter((moduleRoute) => !referencedLazyLoadedModules[moduleRoute.module.fileName]);

function getReferencedModule(loadChildrenModule: string): ts.SourceFile {
const moduleFileName = loadChildrenModule.substring(loadChildrenModule.lastIndexOf('/') + 1);
let referencedModule = moduleRoutesWithLoadChildren.find((moduleRoute) =>
moduleRoute.module.fileName.includes(moduleFileName)
let referencedModule = moduleRoutesWithLoadChildren.find(
(moduleRoute) => moduleRoute.module.fileName === loadChildrenModule
);

// referencedModule may not be found when routing modules are used. In this case the feature module
// imports the feature routing module and the feature routing module is the one that
// contains the RouteModule.forRoot / forChild call
if (!referencedModule) {
const moduleFile = allModuleFiles.find((sourceFile) => sourceFile.fileName.includes(moduleFileName));
const moduleFile = allModuleFiles.find((sourceFile) => sourceFile.fileName === loadChildrenModule);
// In general moduleFile should be found if the naming convention is kept
if (moduleFile) {
// Go through the import statements and get those that contain 'module'
Expand Down Expand Up @@ -301,4 +307,15 @@ function fixupLoadChildren(
}
return referencedModule.module;
}

// Using absolute path prevents issues when two module files have the exact same name
function getLoadChildrenAbsolutePath(moduleFolderPath: string, loadChildrenImportPath: string): string {
// root filenames are user provided and may be relative (eg. see package json's gen-app-routes:tenant)
const absolutePath = path.isAbsolute(moduleFolderPath)
? path.join(moduleFolderPath, loadChildrenImportPath)
: path.join(programDirectory, moduleFolderPath, loadChildrenImportPath);

// Must append the file extension, as the TS imports never include it
return `${absolutePath}.ts`;
}
}

0 comments on commit 15fd4cb

Please sign in to comment.