From 6b3aa60446a9473f5005b86998b79536d4d7b641 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Thu, 2 Apr 2020 16:47:17 +0100 Subject: [PATCH] fix(ngcc): support simple `browser` property in entry-points (#36396) The `browser` package.json property is now supported to the same level as `main` - i.e. it is sniffed for UMD, ESM5 and CommonJS. The `browser` property can also contain an object with file overrides but this is not supported by ngcc. Fixes #36062 PR Close #36396 --- .../ngcc/src/packages/entry_point.ts | 12 ++- .../ngcc/test/integration/ngcc_spec.ts | 2 +- .../ngcc/test/packages/entry_point_spec.ts | 94 +++++++++++-------- 3 files changed, 64 insertions(+), 44 deletions(-) diff --git a/packages/compiler-cli/ngcc/src/packages/entry_point.ts b/packages/compiler-cli/ngcc/src/packages/entry_point.ts index 7a8097c79c..1c6e56fa4f 100644 --- a/packages/compiler-cli/ngcc/src/packages/entry_point.ts +++ b/packages/compiler-cli/ngcc/src/packages/entry_point.ts @@ -50,6 +50,7 @@ export interface JsonObject { } export interface PackageJsonFormatPropertiesMap { + browser?: string; fesm2015?: string; fesm5?: string; es2015?: string; // if exists then it is actually FESM2015 @@ -75,7 +76,7 @@ export interface EntryPointPackageJson extends JsonObject, PackageJsonFormatProp export type EntryPointJsonProperty = Exclude; // We need to keep the elements of this const and the `EntryPointJsonProperty` type in sync. export const SUPPORTED_FORMAT_PROPERTIES: EntryPointJsonProperty[] = - ['fesm2015', 'fesm5', 'es2015', 'esm2015', 'esm5', 'main', 'module']; + ['fesm2015', 'fesm5', 'es2015', 'esm2015', 'esm5', 'main', 'module', 'browser']; /** @@ -193,13 +194,18 @@ export function getEntryPointFormat( return 'esm2015'; case 'esm5': return 'esm5'; + case 'browser': + const browserFile = entryPoint.packageJson['browser']; + if (typeof browserFile !== 'string') { + return undefined; + } + return sniffModuleFormat(fs, join(entryPoint.path, browserFile)); case 'main': const mainFile = entryPoint.packageJson['main']; if (mainFile === undefined) { return undefined; } - const pathToMain = join(entryPoint.path, mainFile); - return sniffModuleFormat(fs, pathToMain); + return sniffModuleFormat(fs, join(entryPoint.path, mainFile)); case 'module': return 'esm5'; default: diff --git a/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts b/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts index 8092d757b7..9e9ff8ea91 100644 --- a/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts +++ b/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts @@ -793,7 +793,7 @@ runInEachFileSystem(() => { const propertiesToConsider = ['es1337', 'fesm42']; const errorMessage = 'No supported format property to consider among [es1337, fesm42]. Supported ' + - 'properties: fesm2015, fesm5, es2015, esm2015, esm5, main, module'; + 'properties: fesm2015, fesm5, es2015, esm2015, esm5, main, module, browser'; expect(() => mainNgcc({basePath: '/node_modules', propertiesToConsider})) .toThrowError(errorMessage); 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 c5b8e143f0..f07c779942 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 {absoluteFrom, AbsoluteFsPath, FileSystem, getFileSystem} from '../../../ import {runInEachFileSystem} from '../../../src/ngtsc/file_system/testing'; import {loadTestFiles} from '../../../test/helpers'; import {NgccConfiguration} from '../../src/packages/configuration'; -import {EntryPoint, getEntryPointFormat, getEntryPointInfo, INCOMPATIBLE_ENTRY_POINT, NO_ENTRY_POINT, SUPPORTED_FORMAT_PROPERTIES} from '../../src/packages/entry_point'; +import {EntryPoint, EntryPointJsonProperty, getEntryPointFormat, getEntryPointInfo, INCOMPATIBLE_ENTRY_POINT, NO_ENTRY_POINT, SUPPORTED_FORMAT_PROPERTIES} from '../../src/packages/entry_point'; import {MockLogger} from '../helpers/mock_logger'; runInEachFileSystem(() => { @@ -206,7 +206,7 @@ runInEachFileSystem(() => { for (let prop of SUPPORTED_FORMAT_PROPERTIES) { // Ignore the UMD format - if (prop === 'main') continue; + if (prop === 'main' || prop === 'browser') continue; // Let's give 'module' a specific path, otherwise compute it based on the property. const typingsPath = prop === 'module' ? 'index' : `${prop}/missing_typings`; @@ -425,67 +425,80 @@ runInEachFileSystem(() => { expect(getEntryPointFormat(fs, entryPoint, 'module')).toBe('esm5'); }); - it('should return `esm5` for `main` if the file contains import or export statements', () => { - const name = _( - '/project/node_modules/some_package/valid_entry_point/bundles/valid_entry_point/index.js'); - loadTestFiles([{name, contents: `import * as core from '@angular/core;`}]); - expect(getEntryPointFormat(fs, entryPoint, 'main')).toBe('esm5'); + (['browser', 'main'] as EntryPointJsonProperty[]).forEach(browserOrMain => { + it('should return `esm5` for `' + browserOrMain + + '` if the file contains import or export statements', + () => { + const name = _( + '/project/node_modules/some_package/valid_entry_point/bundles/valid_entry_point/index.js'); + loadTestFiles([{name, contents: `import * as core from '@angular/core;`}]); + expect(getEntryPointFormat(fs, entryPoint, browserOrMain)).toBe('esm5'); - loadTestFiles([{name, contents: `import {Component} from '@angular/core;`}]); - expect(getEntryPointFormat(fs, entryPoint, 'main')).toBe('esm5'); + loadTestFiles([{name, contents: `import {Component} from '@angular/core;`}]); + expect(getEntryPointFormat(fs, entryPoint, browserOrMain)).toBe('esm5'); - loadTestFiles([{name, contents: `export function foo() {}`}]); - expect(getEntryPointFormat(fs, entryPoint, 'main')).toBe('esm5'); + loadTestFiles([{name, contents: `export function foo() {}`}]); + expect(getEntryPointFormat(fs, entryPoint, browserOrMain)).toBe('esm5'); - loadTestFiles([{name, contents: `export * from 'abc';`}]); - expect(getEntryPointFormat(fs, entryPoint, 'main')).toBe('esm5'); - }); + loadTestFiles([{name, contents: `export * from 'abc';`}]); + expect(getEntryPointFormat(fs, entryPoint, browserOrMain)).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: ` + it('should return `umd` for `' + browserOrMain + + '` 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'); - }); + }]); + expect(getEntryPointFormat(fs, entryPoint, browserOrMain)).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: ` + it('should return `commonjs` for `' + browserOrMain + + '` 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'); - }); + }]); + expect(getEntryPointFormat(fs, entryPoint, browserOrMain)).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: ` + 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/index'; + expect(getEntryPointFormat(fs, entryPoint, browserOrMain)).toBe('umd'); - entryPoint.packageJson.main = './bundles/valid_entry_point'; - expect(getEntryPointFormat(fs, entryPoint, 'main')).toBe('umd'); + entryPoint.packageJson.main = './bundles/valid_entry_point'; + expect(getEntryPointFormat(fs, entryPoint, browserOrMain)).toBe('umd'); + }); + }); + + it('should return `undefined` if the `browser` property is not a string', () => { + entryPoint.packageJson.browser = {} as any; + expect(getEntryPointFormat(fs, entryPoint, 'browser')).toBeUndefined(); }); }); }); @@ -503,6 +516,7 @@ export function createPackageJson( fesm5: `./fesm5/${packageName}.js`, esm5: `./esm5/${packageName}.js`, main: `./bundles/${packageName}/index.js`, + browser: `./bundles/${packageName}/index.js`, module: './index.js', }; if (excludes) {