From db557221bc34bb429cc3d2cbf92b296ff3f43aa8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matias=20Niemel=C3=A4?= Date: Thu, 11 Jul 2019 11:51:13 -0400 Subject: [PATCH] revert: fix(ivy): ngcc - resolve `main` property paths correctly (#31509) This reverts commit 103a5b42ecc7de916ed9815058771ae3e231619e. --- .../ngcc/src/dependencies/module_resolver.ts | 23 ++++- .../src/dependencies/umd_dependency_host.ts | 17 +--- .../ngcc/src/packages/entry_point.ts | 7 +- packages/compiler-cli/ngcc/src/utils.ts | 17 ---- .../dependencies/umd_dependency_host_spec.ts | 10 --- .../ngcc/test/packages/entry_point_spec.ts | 90 +------------------ 6 files changed, 27 insertions(+), 137 deletions(-) diff --git a/packages/compiler-cli/ngcc/src/dependencies/module_resolver.ts b/packages/compiler-cli/ngcc/src/dependencies/module_resolver.ts index 1208744b98..549690a092 100644 --- a/packages/compiler-cli/ngcc/src/dependencies/module_resolver.ts +++ b/packages/compiler-cli/ngcc/src/dependencies/module_resolver.ts @@ -5,8 +5,11 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ + import {AbsoluteFsPath, FileSystem, absoluteFrom, dirname, isRoot, join, resolve} from '../../../src/ngtsc/file_system'; -import {PathMappings, isRelativePath, resolveFileWithPostfixes} from '../utils'; +import {PathMappings, isRelativePath} from '../utils'; + + /** * This is a very cut-down implementation of the TypeScript module resolution strategy. @@ -69,8 +72,8 @@ export class ModuleResolver { * If neither of these files exist then the method returns `null`. */ private resolveAsRelativePath(moduleName: string, fromPath: AbsoluteFsPath): ResolvedModule|null { - const resolvedPath = resolveFileWithPostfixes( - this.fs, resolve(dirname(fromPath), moduleName), this.relativeExtensions); + const resolvedPath = + this.resolvePath(resolve(dirname(fromPath), moduleName), this.relativeExtensions); return resolvedPath && new ResolvedRelativeModule(resolvedPath); } @@ -130,6 +133,20 @@ export class ModuleResolver { return null; } + /** + * Attempt to resolve a `path` to a file by appending the provided `postFixes` + * to the `path` and checking if the file exists on disk. + * @returns An absolute path to the first matching existing file, or `null` if none exist. + */ + private resolvePath(path: AbsoluteFsPath, postFixes: string[]): AbsoluteFsPath|null { + for (const postFix of postFixes) { + const testPath = absoluteFrom(path + postFix); + if (this.fs.exists(testPath)) { + return testPath; + } + } + return null; + } /** * Can we consider the given path as an entry-point to a package? diff --git a/packages/compiler-cli/ngcc/src/dependencies/umd_dependency_host.ts b/packages/compiler-cli/ngcc/src/dependencies/umd_dependency_host.ts index dc8c6e298d..7828e3db88 100644 --- a/packages/compiler-cli/ngcc/src/dependencies/umd_dependency_host.ts +++ b/packages/compiler-cli/ngcc/src/dependencies/umd_dependency_host.ts @@ -6,15 +6,11 @@ * found in the LICENSE file at https://angular.io/license */ import * as ts from 'typescript'; - import {AbsoluteFsPath, FileSystem, PathSegment} from '../../../src/ngtsc/file_system'; import {getImportsOfUmdModule, parseStatementForUmdModule} from '../host/umd_host'; -import {resolveFileWithPostfixes} from '../utils'; - import {DependencyHost, DependencyInfo} from './dependency_host'; import {ModuleResolver, ResolvedDeepImport, ResolvedRelativeModule} from './module_resolver'; - /** * Helper functions for computing dependencies. */ @@ -54,20 +50,15 @@ export class UmdDependencyHost implements DependencyHost { private recursivelyFindDependencies( file: AbsoluteFsPath, dependencies: Set, missing: Set, deepImports: Set, alreadySeen: Set): void { - const resolvedFile = resolveFileWithPostfixes(this.fs, file, ['', '.js', '/index.js']); - if (resolvedFile === null) { - return; - } - - const fromContents = this.fs.readFile(resolvedFile); + const fromContents = this.fs.readFile(file); if (!this.hasRequireCalls(fromContents)) { // Avoid parsing the source file as there are no require calls. return; } // Parse the source into a TypeScript AST and then walk it looking for imports and re-exports. - const sf = ts.createSourceFile( - resolvedFile, fromContents, ts.ScriptTarget.ES2015, false, ts.ScriptKind.JS); + const sf = + ts.createSourceFile(file, fromContents, ts.ScriptTarget.ES2015, false, ts.ScriptKind.JS); if (sf.statements.length !== 1) { return; } @@ -79,7 +70,7 @@ export class UmdDependencyHost implements DependencyHost { } umdImports.forEach(umdImport => { - const resolvedModule = this.moduleResolver.resolveModuleImport(umdImport.path, resolvedFile); + const resolvedModule = this.moduleResolver.resolveModuleImport(umdImport.path, file); if (resolvedModule) { if (resolvedModule instanceof ResolvedRelativeModule) { const internalDependency = resolvedModule.modulePath; diff --git a/packages/compiler-cli/ngcc/src/packages/entry_point.ts b/packages/compiler-cli/ngcc/src/packages/entry_point.ts index 10b6785fc0..2c043d210f 100644 --- a/packages/compiler-cli/ngcc/src/packages/entry_point.ts +++ b/packages/compiler-cli/ngcc/src/packages/entry_point.ts @@ -11,7 +11,6 @@ import * as ts from 'typescript'; import {AbsoluteFsPath, FileSystem, join, resolve} from '../../../src/ngtsc/file_system'; import {parseStatementForUmdModule} from '../host/umd_host'; import {Logger} from '../logging/logger'; -import {resolveFileWithPostfixes} from '../utils'; import {NgccConfiguration, NgccEntryPointConfig} from './configuration'; /** @@ -168,12 +167,8 @@ function loadEntryPointPackage( } function isUmdModule(fs: FileSystem, sourceFilePath: AbsoluteFsPath): boolean { - const resolvedPath = resolveFileWithPostfixes(fs, sourceFilePath, ['', '.js', '/index.js']); - if (resolvedPath === null) { - return false; - } const sourceFile = - ts.createSourceFile(sourceFilePath, fs.readFile(resolvedPath), ts.ScriptTarget.ES5); + ts.createSourceFile(sourceFilePath, fs.readFile(sourceFilePath), ts.ScriptTarget.ES5); return sourceFile.statements.length > 0 && parseStatementForUmdModule(sourceFile.statements[0]) !== null; } diff --git a/packages/compiler-cli/ngcc/src/utils.ts b/packages/compiler-cli/ngcc/src/utils.ts index c3eb08378b..9cb91a2c09 100644 --- a/packages/compiler-cli/ngcc/src/utils.ts +++ b/packages/compiler-cli/ngcc/src/utils.ts @@ -6,7 +6,6 @@ * found in the LICENSE file at https://angular.io/license */ import * as ts from 'typescript'; -import {AbsoluteFsPath, FileSystem, absoluteFrom} from '../../src/ngtsc/file_system'; export function getOriginalSymbol(checker: ts.TypeChecker): (symbol: ts.Symbol) => ts.Symbol { return function(symbol: ts.Symbol) { @@ -66,19 +65,3 @@ export type PathMappings = { export function isRelativePath(path: string): boolean { return /^\/|^\.\.?($|\/)/.test(path); } - -/** - * Attempt to resolve a `path` to a file by appending the provided `postFixes` - * to the `path` and checking if the file exists on disk. - * @returns An absolute path to the first matching existing file, or `null` if none exist. - */ -export function resolveFileWithPostfixes( - fs: FileSystem, path: AbsoluteFsPath, postFixes: string[]): AbsoluteFsPath|null { - for (const postFix of postFixes) { - const testPath = absoluteFrom(path + postFix); - if (fs.exists(testPath) && fs.stat(testPath).isFile()) { - return testPath; - } - } - return null; -} diff --git a/packages/compiler-cli/ngcc/test/dependencies/umd_dependency_host_spec.ts b/packages/compiler-cli/ngcc/test/dependencies/umd_dependency_host_spec.ts index 6f8e3f64fa..2dd04369e5 100644 --- a/packages/compiler-cli/ngcc/test/dependencies/umd_dependency_host_spec.ts +++ b/packages/compiler-cli/ngcc/test/dependencies/umd_dependency_host_spec.ts @@ -115,16 +115,6 @@ runInEachFileSystem(() => { expect(missing.size).toBe(0); expect(deepImports.size).toBe(0); }); - - it('should resolve path to the file if it has no extension', () => { - const {dependencies, missing, deepImports} = - host.findDependencies(_('/external/imports/index')); - expect(dependencies.size).toBe(2); - expect(missing.size).toBe(0); - expect(deepImports.size).toBe(0); - expect(dependencies.has(_('/node_modules/lib_1'))).toBe(true); - expect(dependencies.has(_('/node_modules/lib_1/sub_1'))).toBe(true); - }); }); function setupMockFileSystem(): void { diff --git a/packages/compiler-cli/ngcc/test/packages/entry_point_spec.ts b/packages/compiler-cli/ngcc/test/packages/entry_point_spec.ts index 968c3acc03..984379a11b 100644 --- a/packages/compiler-cli/ngcc/test/packages/entry_point_spec.ts +++ b/packages/compiler-cli/ngcc/test/packages/entry_point_spec.ts @@ -10,7 +10,7 @@ import {AbsoluteFsPath, FileSystem, absoluteFrom, getFileSystem} from '../../../ import {runInEachFileSystem} from '../../../src/ngtsc/file_system/testing'; import {loadTestFiles} from '../../../test/helpers'; import {NgccConfiguration} from '../../src/packages/configuration'; -import {EntryPoint, SUPPORTED_FORMAT_PROPERTIES, getEntryPointFormat, getEntryPointInfo} from '../../src/packages/entry_point'; +import {SUPPORTED_FORMAT_PROPERTIES, getEntryPointInfo} from '../../src/packages/entry_point'; import {MockLogger} from '../helpers/mock_logger'; runInEachFileSystem(() => { @@ -340,92 +340,6 @@ runInEachFileSystem(() => { }); }); - describe('getEntryPointFormat', () => { - let SOME_PACKAGE: AbsoluteFsPath; - let _: typeof absoluteFrom; - let fs: FileSystem; - let entryPoint: EntryPoint; - - beforeEach(() => { - _ = absoluteFrom; - SOME_PACKAGE = _('/project/node_modules/some_package'); - fs = getFileSystem(); - loadTestFiles([{ - name: _('/project/node_modules/some_package/valid_entry_point/package.json'), - contents: createPackageJson('valid_entry_point') - }]); - const config = new NgccConfiguration(fs, _('/project')); - entryPoint = getEntryPointInfo( - fs, config, new MockLogger(), SOME_PACKAGE, - _('/project/node_modules/some_package/valid_entry_point')) !; - }); - - it('should return `esm2015` format for `fesm2015` property', - () => { expect(getEntryPointFormat(fs, entryPoint, 'fesm2015')).toBe('esm2015'); }); - - it('should return `esm5` format for `fesm5` property', - () => { expect(getEntryPointFormat(fs, entryPoint, 'fesm5')).toBe('esm5'); }); - - it('should return `esm2015` format for `es2015` property', - () => { expect(getEntryPointFormat(fs, entryPoint, 'es2015')).toBe('esm2015'); }); - - it('should return `esm2015` format for `esm2015` property', - () => { expect(getEntryPointFormat(fs, entryPoint, 'esm2015')).toBe('esm2015'); }); - - it('should return `esm5` format for `esm5` property', - () => { expect(getEntryPointFormat(fs, entryPoint, 'esm5')).toBe('esm5'); }); - - it('should return `esm5` format for `module` property', - () => { expect(getEntryPointFormat(fs, entryPoint, 'module')).toBe('esm5'); }); - - it('should return `umd` for `main` if the file contains a UMD wrapper function', () => { - loadTestFiles([{ - name: _( - '/project/node_modules/some_package/valid_entry_point/bundles/valid_entry_point/index.js'), - contents: ` - (function (global, factory) { - typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports, require('@angular/core')) : - typeof define === 'function' && define.amd ? define('@angular/common', ['exports', '@angular/core'], factory) : - (global = global || self, factory((global.ng = global.ng || {}, global.ng.common = {}), global.ng.core)); - }(this, function (exports, core) { 'use strict'; })); - ` - }]); - expect(getEntryPointFormat(fs, entryPoint, 'main')).toBe('umd'); - }); - - it('should return `commonjs` for `main` if the file does not contain a UMD wrapper function', () => { - loadTestFiles([{ - name: _( - '/project/node_modules/some_package/valid_entry_point/bundles/valid_entry_point/index.js'), - contents: ` - const core = require('@angular/core); - module.exports = {}; - ` - }]); - expect(getEntryPointFormat(fs, entryPoint, 'main')).toBe('commonjs'); - }); - - it('should resolve the format path with suitable postfixes', () => { - loadTestFiles([{ - name: _( - '/project/node_modules/some_package/valid_entry_point/bundles/valid_entry_point/index.js'), - contents: ` - (function (global, factory) { - typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports, require('@angular/core')) : - typeof define === 'function' && define.amd ? define('@angular/common', ['exports', '@angular/core'], factory) : - (global = global || self, factory((global.ng = global.ng || {}, global.ng.common = {}), global.ng.core)); - }(this, function (exports, core) { 'use strict'; })); - ` - }]); - - entryPoint.packageJson.main = './bundles/valid_entry_point/index'; - expect(getEntryPointFormat(fs, entryPoint, 'main')).toBe('umd'); - - entryPoint.packageJson.main = './bundles/valid_entry_point'; - expect(getEntryPointFormat(fs, entryPoint, 'main')).toBe('umd'); - }); - }); - function createPackageJson( packageName: string, {excludes}: {excludes?: string[]} = {}, typingsProp: string = 'typings'): string { @@ -437,7 +351,7 @@ runInEachFileSystem(() => { es2015: `./es2015/${packageName}.js`, fesm5: `./fesm5/${packageName}.js`, esm5: `./esm5/${packageName}.js`, - main: `./bundles/${packageName}/index.js`, + main: `./bundles/${packageName}.umd.js`, module: './index.js', }; if (excludes) {