Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
fix(compiler-cli): incorrectly generating relative file paths on case…
…-insensitive platforms

We're using `path.relative` to compute a relative path between a `SourceFile` and the one of the `rootDirs`. The problem is that the `rootDirs` get passed through `getCanonicalFileName` which lowercases the path in some platforms, while `SourceFile.fileName` is always case-insensitive. This yields a path outside of the project which we were ignoring.

This change passes the `SourceFile.fileName` before passing it through `path.relative` to ensure that we get a valid result.
  • Loading branch information
crisbeto committed Oct 10, 2024
commit 4f683078ebc02f3c27af5d1874cb4f2b29f890cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,15 @@
*/

import {literal, R3ClassDebugInfo, WrappedNodeExpr} from '@angular/compiler';
import * as path from 'path';
import ts from 'typescript';

import {DeclarationNode, ReflectionHost} from '../../../reflection';
import {getProjectRelativePath} from './util';

export function extractClassDebugInfo(
clazz: DeclarationNode,
reflection: ReflectionHost,
compilerHost: Pick<ts.CompilerHost, 'getCanonicalFileName'>,
rootDirs: ReadonlyArray<string>,
forbidOrphanRendering: boolean,
): R3ClassDebugInfo | null {
Expand All @@ -22,7 +24,7 @@ export function extractClassDebugInfo(
}

const srcFile = clazz.getSourceFile();
const srcFileMaybeRelativePath = computeRelativePathIfPossible(srcFile.fileName, rootDirs);
const srcFileMaybeRelativePath = getProjectRelativePath(srcFile, rootDirs, compilerHost);

return {
type: new WrappedNodeExpr(clazz.name),
Expand All @@ -32,21 +34,3 @@ export function extractClassDebugInfo(
forbidOrphanRendering,
};
}

/**
* Computes a source file path relative to the project root folder if possible, otherwise returns
* null.
*/
function computeRelativePathIfPossible(
filePath: string,
rootDirs: ReadonlyArray<string>,
): string | null {
for (const rootDir of rootDirs) {
const rel = path.relative(rootDir, filePath);
if (!rel.startsWith('..')) {
return rel;
}
}

return null;
}
29 changes: 29 additions & 0 deletions packages/compiler-cli/src/ngtsc/annotations/common/src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
Statement,
WrappedNodeExpr,
} from '@angular/compiler';
import {relative} from 'path';
import ts from 'typescript';

import {
Expand Down Expand Up @@ -509,3 +510,31 @@ export function isAbstractClassDeclaration(clazz: ClassDeclaration): boolean {
? clazz.modifiers.some((mod) => mod.kind === ts.SyntaxKind.AbstractKeyword)
: false;
}

/**
* Attempts to generate a project-relative path
* @param sourceFile
* @param rootDirs
* @param compilerHost
* @returns
*/
export function getProjectRelativePath(
sourceFile: ts.SourceFile,
rootDirs: readonly string[],
compilerHost: Pick<ts.CompilerHost, 'getCanonicalFileName'>,
): string | null {
// Note: we need to pass both the file name and the root directories through getCanonicalFileName,
// because the root directories might've been passed through it already while the source files
// definitely have not. This can break the relative return value, because in some platforms
// getCanonicalFileName lowercases the path.
const filePath = compilerHost.getCanonicalFileName(sourceFile.fileName);

for (const rootDir of rootDirs) {
const rel = relative(compilerHost.getCanonicalFileName(rootDir), filePath);
if (!rel.startsWith('..')) {
return rel;
}
}

return null;
}
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ export class ComponentDecoratorHandler
private metaRegistry: MetadataRegistry,
private metaReader: MetadataReader,
private scopeReader: ComponentScopeReader,
private dtsScopeReader: DtsModuleScopeResolver,
private compilerHost: Pick<ts.CompilerHost, 'getCanonicalFileName'>,
private scopeRegistry: LocalModuleScopeRegistry,
private typeCheckScopeRegistry: TypeCheckScopeRegistry,
private resourceRegistry: ResourceRegistry,
Expand Down Expand Up @@ -872,11 +872,12 @@ export class ComponentDecoratorHandler
classDebugInfo: extractClassDebugInfo(
node,
this.reflector,
this.compilerHost,
this.rootDirs,
/* forbidOrphanRenderering */ this.forbidOrphanRendering,
),
hmrInitializerMeta: this.enableHmr
? extractHmrInitializerMeta(node, this.reflector, this.rootDirs)
? extractHmrInitializerMeta(node, this.reflector, this.compilerHost, this.rootDirs)
: null,
template,
providersRequiringFactory,
Expand Down
17 changes: 7 additions & 10 deletions packages/compiler-cli/src/ngtsc/annotations/component/src/hmr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,29 +8,26 @@

import {R3HmrInitializerMetadata, WrappedNodeExpr} from '@angular/compiler';
import {DeclarationNode, ReflectionHost} from '../../../reflection';
import {relative} from 'path';
import {getProjectRelativePath} from '../../common';
import ts from 'typescript';

/**
* Extracts the metadata necessary to generate an HMR initializer.
*/
export function extractHmrInitializerMeta(
clazz: DeclarationNode,
reflection: ReflectionHost,
compilerHost: Pick<ts.CompilerHost, 'getCanonicalFileName'>,
rootDirs: readonly string[],
): R3HmrInitializerMetadata | null {
if (!reflection.isClass(clazz)) {
return null;
}

// Attempt to generate a project-relative path before falling back to the full path.
let filePath = clazz.getSourceFile().fileName;
for (const rootDir of rootDirs) {
const relativePath = relative(rootDir, filePath);
if (!relativePath.startsWith('..')) {
filePath = relativePath;
break;
}
}
const sourceFile = clazz.getSourceFile();
const filePath =
getProjectRelativePath(sourceFile, rootDirs, compilerHost) ||
compilerHost.getCanonicalFileName(sourceFile.fileName);

const meta: R3HmrInitializerMetadata = {
type: new WrappedNodeExpr(clazz.name),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,9 @@ function setup(
metaRegistry,
metaReader,
scopeRegistry,
dtsResolver,
{
getCanonicalFileName: (fileName) => fileName,
},
scopeRegistry,
typeCheckScopeRegistry,
resourceRegistry,
Expand Down
2 changes: 1 addition & 1 deletion packages/compiler-cli/src/ngtsc/core/src/compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1429,7 +1429,7 @@ export class NgCompiler {
metaRegistry,
metaReader,
scopeReader,
depScopeReader,
this.adapter,
ngModuleScopeRegistry,
typeCheckScopeRegistry,
resourceRegistry,
Expand Down
4 changes: 2 additions & 2 deletions packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10574,10 +10574,10 @@ runInEachFileSystem((os: string) => {
// We need a regex match here, because the file path changes based on
// the file system and the timestamp will be different for each test run.
expect(jsContents).toMatch(
/import\.meta\.hot && import\.meta\.hot\.on\("angular:component-update", d => { if \(d\.id == ".*test\.ts%40Cmp"\) {/,
/import\.meta\.hot && import\.meta\.hot\.on\("angular:component-update", d => { if \(d\.id == "test\.ts%40Cmp"\) {/,
);
expect(jsContents).toMatch(
/import\("\/@ng\/component\?c=.*test\.ts%40Cmp&t=\d+"\).then\(m => i0\.ɵɵreplaceMetadata\(Cmp, m\.default\)\);/,
/import\("\/@ng\/component\?c=test\.ts%40Cmp&t=\d+"\).then\(m => i0\.ɵɵreplaceMetadata\(Cmp, m\.default\)\);/,
);
});
});
Expand Down