fix(ngcc): correctly identify relative Windows-style import paths (#36372)

Previously, `isRelativePath()` assumed paths are *nix-style. This caused
Windows-style paths (such as `C:\foo\some-package\some-file.js`) to not
be recognized as "relative" imports.

This commit fixes this by using the OS-agnostic `isRooted()` helper and
also accounting for both styles of path delimiters: `/` and `\`

PR Close #36372
This commit is contained in:
George Kalpakas 2020-04-01 17:31:04 +03:00 committed by Kara Erickson
parent 81195a238b
commit aecf9de738
5 changed files with 58 additions and 29 deletions

View File

@ -7,7 +7,7 @@
*/
import * as ts from 'typescript';
import {absoluteFrom, AbsoluteFsPath, FileSystem} from '../../src/ngtsc/file_system';
import {absoluteFrom, AbsoluteFsPath, FileSystem, isRooted} from '../../src/ngtsc/file_system';
import {KnownDeclaration} from '../../src/ngtsc/reflection';
/**
@ -85,10 +85,11 @@ export type PathMappings = {
/**
* Test whether a path is "relative".
*
* Relative paths start with `/`, `./` or `../`; or are simply `.` or `..`.
* Relative paths start with `/`, `./` or `../` (or the Windows equivalents); or are simply `.` or
* `..`.
*/
export function isRelativePath(path: string): boolean {
return /^\/|^\.\.?($|\/)/.test(path);
return isRooted(path) || /^\.\.?(\/|\\|$)/.test(path);
}
/**

View File

@ -67,16 +67,29 @@ runInEachFileSystem(() => {
]);
});
describe('resolveModule()', () => {
describe('resolveModuleImport()', () => {
describe('with relative paths', () => {
it('should resolve sibling, child and aunt modules', () => {
const resolver = new ModuleResolver(getFileSystem());
// With relative file paths.
expect(resolver.resolveModuleImport('./x', _('/libs/local-package/index.js')))
.toEqual(new ResolvedRelativeModule(_('/libs/local-package/x.js')));
expect(resolver.resolveModuleImport('./sub-folder', _('/libs/local-package/index.js')))
.toEqual(new ResolvedRelativeModule(_('/libs/local-package/sub-folder/index.js')));
expect(resolver.resolveModuleImport('../x', _('/libs/local-package/sub-folder/index.js')))
.toEqual(new ResolvedRelativeModule(_('/libs/local-package/x.js')));
// With absolute file paths.
expect(resolver.resolveModuleImport(
_('/libs/local-package/x'), _('/libs/local-package/index.js')))
.toEqual(new ResolvedRelativeModule(_('/libs/local-package/x.js')));
expect(resolver.resolveModuleImport(
_('/libs/local-package/sub-folder'), _('/libs/local-package/index.js')))
.toEqual(new ResolvedRelativeModule(_('/libs/local-package/sub-folder/index.js')));
expect(resolver.resolveModuleImport(
_('/libs/local-package/x'), _('/libs/local-package/sub-folder/index.js')))
.toEqual(new ResolvedRelativeModule(_('/libs/local-package/x.js')));
});
it('should return `null` if the resolved module relative module does not exist', () => {

View File

@ -7,6 +7,8 @@
*/
import * as ts from 'typescript';
import {absoluteFrom as _abs} from '../../src/ngtsc/file_system';
import {runInEachFileSystem} from '../../src/ngtsc/file_system/testing';
import {KnownDeclaration} from '../../src/ngtsc/reflection';
import {FactoryMap, getTsHelperFnFromDeclaration, getTsHelperFnFromIdentifier, isRelativePath, stripExtension} from '../src/utils';
@ -167,19 +169,24 @@ describe('getTsHelperFnFromIdentifier()', () => {
});
});
runInEachFileSystem(() => {
describe('isRelativePath()', () => {
it('should return true for relative paths', () => {
expect(isRelativePath('.')).toBe(true);
expect(isRelativePath('..')).toBe(true);
expect(isRelativePath('./')).toBe(true);
expect(isRelativePath('.\\')).toBe(true);
expect(isRelativePath('../')).toBe(true);
expect(isRelativePath('..\\')).toBe(true);
expect(isRelativePath('./abc/xyz')).toBe(true);
expect(isRelativePath('.\\abc\\xyz')).toBe(true);
expect(isRelativePath('../abc/xyz')).toBe(true);
expect(isRelativePath('..\\abc\\xyz')).toBe(true);
});
it('should return true for absolute paths', () => {
expect(isRelativePath('/')).toBe(true);
expect(isRelativePath('/abc/xyz')).toBe(true);
expect(isRelativePath(_abs('/'))).toBe(true);
expect(isRelativePath(_abs('/abc/xyz'))).toBe(true);
});
it('should return false for other paths', () => {
@ -193,6 +200,7 @@ describe('isRelativePath()', () => {
expect(isRelativePath('@abc/xyz')).toBe(false);
});
});
});
describe('stripExtension()', () => {
it('should strip the extension from a file name', () => {

View File

@ -7,7 +7,7 @@
*/
export {CachedFileSystem} from './src/cached_file_system';
export {NgtscCompilerHost} from './src/compiler_host';
export {absoluteFrom, absoluteFromSourceFile, basename, dirname, getFileSystem, isRoot, join, relative, relativeFrom, resolve, setFileSystem} from './src/helpers';
export {absoluteFrom, absoluteFromSourceFile, basename, dirname, getFileSystem, isRoot, isRooted, join, relative, relativeFrom, resolve, setFileSystem} from './src/helpers';
export {LogicalFileSystem, LogicalProjectPath} from './src/logical';
export {NodeJSFileSystem} from './src/node_js_file_system';
export {AbsoluteFsPath, FileStats, FileSystem, PathSegment, PathString} from './src/types';

View File

@ -73,6 +73,13 @@ export function isRoot(path: AbsoluteFsPath): boolean {
return fs.isRoot(path);
}
/**
* Static access to `isRooted`.
*/
export function isRooted(path: string): boolean {
return fs.isRooted(path);
}
/**
* Static access to `relative`.
*/