From d0037b22ef1e6879545bffdf013b5b32917821af Mon Sep 17 00:00:00 2001 From: JoostK Date: Wed, 24 Oct 2018 18:39:29 +0200 Subject: [PATCH] fix(ivy): correct ngtsc path handling in Windows (#26703) As it turns out, the usage of path.posix does not unify path handling across operating systems. Instead, canonical-path is used to ensure path handling is consistent, avoiding incorrect paths in Windows. See https://github.com/angular/angular/pull/25862#discussion_r216157914 PR Close #26703 --- packages/compiler-cli/src/ngtsc/annotations/BUILD.bazel | 1 + .../compiler-cli/src/ngtsc/annotations/src/component.ts | 8 ++++---- packages/compiler-cli/src/ngtsc/metadata/BUILD.bazel | 1 + packages/compiler-cli/src/ngtsc/metadata/src/resolver.ts | 4 ++-- packages/compiler-cli/src/ngtsc/shims/BUILD.bazel | 1 + .../compiler-cli/src/ngtsc/shims/src/factory_generator.ts | 5 ++--- packages/compiler-cli/src/ngtsc/testing/BUILD.bazel | 1 + .../src/ngtsc/testing/in_memory_typescript.ts | 4 ++-- packages/compiler-cli/src/ngtsc/util/BUILD.bazel | 2 +- packages/compiler-cli/src/ngtsc/util/src/path.ts | 6 +++--- 10 files changed, 18 insertions(+), 15 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/annotations/BUILD.bazel b/packages/compiler-cli/src/ngtsc/annotations/BUILD.bazel index e86b8e6181..60a064fff8 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/annotations/BUILD.bazel @@ -17,6 +17,7 @@ ts_library( "//packages/compiler-cli/src/ngtsc/transform", "//packages/compiler-cli/src/ngtsc/typecheck", "@ngdeps//@types/node", + "@ngdeps//canonical-path", "@ngdeps//typescript", ], ) diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts index f1ecb66353..b093bb5aa4 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts @@ -7,7 +7,7 @@ */ import {ConstantPool, CssSelector, Expression, R3ComponentMetadata, R3DirectiveMetadata, SelectorMatcher, Statement, TmplAstNode, WrappedNodeExpr, compileComponentFromMetadata, makeBindingParser, parseTemplate} from '@angular/compiler'; -import * as path from 'path'; +import * as path from 'canonical-path'; import * as ts from 'typescript'; import {ErrorCode, FatalDiagnosticError} from '../../diagnostics'; @@ -62,7 +62,7 @@ export class ComponentDecoratorHandler implements throw new FatalDiagnosticError( ErrorCode.VALUE_HAS_WRONG_TYPE, templateUrlExpr, 'templateUrl must be a string'); } - const url = path.posix.resolve(path.dirname(node.getSourceFile().fileName), templateUrl); + const url = path.resolve(path.dirname(node.getSourceFile().fileName), templateUrl); return this.resourceLoader.preload(url); } return undefined; @@ -94,7 +94,7 @@ export class ComponentDecoratorHandler implements throw new FatalDiagnosticError( ErrorCode.VALUE_HAS_WRONG_TYPE, templateUrlExpr, 'templateUrl must be a string'); } - const url = path.posix.resolve(path.dirname(node.getSourceFile().fileName), templateUrl); + const url = path.resolve(path.dirname(node.getSourceFile().fileName), templateUrl); templateStr = this.resourceLoader.load(url); } else if (component.has('template')) { const templateExpr = component.get('template') !; @@ -128,7 +128,7 @@ export class ComponentDecoratorHandler implements // relative path representation. const filePath = node.getSourceFile().fileName; const relativeFilePath = this.rootDirs.reduce((previous, rootDir) => { - const candidate = path.posix.relative(rootDir, filePath); + const candidate = path.relative(rootDir, filePath); if (previous === undefined || candidate.length < previous.length) { return candidate; } else { diff --git a/packages/compiler-cli/src/ngtsc/metadata/BUILD.bazel b/packages/compiler-cli/src/ngtsc/metadata/BUILD.bazel index 530adc1b09..312d9279db 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/metadata/BUILD.bazel @@ -15,6 +15,7 @@ ts_library( "//packages/compiler-cli/src/ngtsc/host", "//packages/compiler-cli/src/ngtsc/util", "@ngdeps//@types/node", + "@ngdeps//canonical-path", "@ngdeps//typescript", ], ) diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/resolver.ts b/packages/compiler-cli/src/ngtsc/metadata/src/resolver.ts index 60675469ea..44362dcf93 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/resolver.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/resolver.ts @@ -12,7 +12,7 @@ */ import {Expression, ExternalExpr, ExternalReference, WrappedNodeExpr} from '@angular/compiler'; -import * as path from 'path'; +import * as path from 'canonical-path'; import * as ts from 'typescript'; import {ClassMemberKind, ReflectionHost} from '../../host'; @@ -155,7 +155,7 @@ export class ResolvedReference extends Reference // TODO(alxhub): investigate the impact of multiple source roots here. // TODO(alxhub): investigate the need to map such paths via the Host for proper g3 support. let relative = - path.posix.relative(path.dirname(context.fileName), this.node.getSourceFile().fileName) + path.relative(path.dirname(context.fileName), this.node.getSourceFile().fileName) .replace(TS_DTS_JS_EXTENSION, ''); // path.relative() does not include the leading './'. diff --git a/packages/compiler-cli/src/ngtsc/shims/BUILD.bazel b/packages/compiler-cli/src/ngtsc/shims/BUILD.bazel index 68e66fb562..2b2630c384 100644 --- a/packages/compiler-cli/src/ngtsc/shims/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/shims/BUILD.bazel @@ -15,6 +15,7 @@ ts_library( "//packages/compiler-cli/src/ngtsc/metadata", "//packages/compiler-cli/src/ngtsc/util", "@ngdeps//@types/node", + "@ngdeps//canonical-path", "@ngdeps//typescript", ], ) diff --git a/packages/compiler-cli/src/ngtsc/shims/src/factory_generator.ts b/packages/compiler-cli/src/ngtsc/shims/src/factory_generator.ts index de102329b1..ccd2a0d1e6 100644 --- a/packages/compiler-cli/src/ngtsc/shims/src/factory_generator.ts +++ b/packages/compiler-cli/src/ngtsc/shims/src/factory_generator.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import * as path from 'path'; +import * as path from 'canonical-path'; import * as ts from 'typescript'; import {relativePathBetween} from '../../util/src/path'; @@ -29,8 +29,7 @@ export class FactoryGenerator implements ShimGenerator { getOriginalSourceOfShim(fileName: string): string|null { return this.map.get(fileName) || null; } generate(original: ts.SourceFile, genFilePath: string): ts.SourceFile { - const relativePathToSource = - './' + path.posix.basename(original.fileName).replace(TS_DTS_SUFFIX, ''); + const relativePathToSource = './' + path.basename(original.fileName).replace(TS_DTS_SUFFIX, ''); // Collect a list of classes that need to have factory types emitted for them. This list is // overly broad as at this point the ts.TypeChecker hasn't been created, and can't be used to // semantically understand which decorated types are actually decorated with Angular decorators. diff --git a/packages/compiler-cli/src/ngtsc/testing/BUILD.bazel b/packages/compiler-cli/src/ngtsc/testing/BUILD.bazel index db42c39981..c561b41b1a 100644 --- a/packages/compiler-cli/src/ngtsc/testing/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/testing/BUILD.bazel @@ -10,6 +10,7 @@ ts_library( ]), deps = [ "//packages:types", + "@ngdeps//canonical-path", "@ngdeps//typescript", ], ) diff --git a/packages/compiler-cli/src/ngtsc/testing/in_memory_typescript.ts b/packages/compiler-cli/src/ngtsc/testing/in_memory_typescript.ts index 36a5b90b0d..45b8a3d0a5 100644 --- a/packages/compiler-cli/src/ngtsc/testing/in_memory_typescript.ts +++ b/packages/compiler-cli/src/ngtsc/testing/in_memory_typescript.ts @@ -8,7 +8,7 @@ /// -import * as path from 'path'; +import * as path from 'canonical-path'; import * as ts from 'typescript'; export function makeProgram( @@ -93,7 +93,7 @@ export class InMemoryHost implements ts.CompilerHost { } getCanonicalFileName(fileName: string): string { - return path.posix.normalize(`${this.getCurrentDirectory()}/${fileName}`); + return path.normalize(`${this.getCurrentDirectory()}/${fileName}`); } useCaseSensitiveFileNames(): boolean { return true; } diff --git a/packages/compiler-cli/src/ngtsc/util/BUILD.bazel b/packages/compiler-cli/src/ngtsc/util/BUILD.bazel index 9deabafc08..2938388d0f 100644 --- a/packages/compiler-cli/src/ngtsc/util/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/util/BUILD.bazel @@ -5,13 +5,13 @@ load("//tools:defaults.bzl", "ts_library") ts_library( name = "util", srcs = glob([ - "index.ts", "src/**/*.ts", ]), module_name = "@angular/compiler-cli/src/ngtsc/util", deps = [ "//packages:types", "@ngdeps//@types/node", + "@ngdeps//canonical-path", "@ngdeps//typescript", ], ) diff --git a/packages/compiler-cli/src/ngtsc/util/src/path.ts b/packages/compiler-cli/src/ngtsc/util/src/path.ts index 954eba6777..9249304ed3 100644 --- a/packages/compiler-cli/src/ngtsc/util/src/path.ts +++ b/packages/compiler-cli/src/ngtsc/util/src/path.ts @@ -8,12 +8,12 @@ /// -import * as path from 'path'; +import * as path from 'canonical-path'; const TS_DTS_EXTENSION = /(\.d)?\.ts$/; export function relativePathBetween(from: string, to: string): string|null { - let relative = path.posix.relative(path.dirname(from), to).replace(TS_DTS_EXTENSION, ''); + let relative = path.relative(path.dirname(from), to).replace(TS_DTS_EXTENSION, ''); if (relative === '') { return null; @@ -25,4 +25,4 @@ export function relativePathBetween(from: string, to: string): string|null { } return relative; -} \ No newline at end of file +}