From d5293d2aa3fe7e79f4ebb5de5fc52a5d70b91aef Mon Sep 17 00:00:00 2001
From: Paul Gschwendtner <paulgschwendtner@gmail.com>
Date: Wed, 6 May 2020 12:45:26 +0200
Subject: [PATCH] fix(bazel): ng_package rule should update "package.json" of
 ts_library targets (#36944)

In the past we added support for `ts_library` to `ng_package`. For those
targets we never can determine the "index" file. Unlike `ng_module`,
there is no provider data for flat module bundles, so the `ng_package`
rule assumes that the index file is simply called `index`.

This works as expected, but we also added logic in the past that doesn't
allow `ng_package` to add format properties (e.g. `main`, `module`) to a
`package.json` if a package json is handwritten for such a `ts_library` target.

This has been done that way because we assumed that such `package.json` files
might want to set format properties explicitly to different paths due to a
faulty "index" guess.

We want to change this behavior as most of the time a `package.json`
file already exists with just the module name. In those cases, the
packager should still set the format properties. We should only warn
and skip automatic insertion of the format properties if such a
`package.json` explicitly sets format properties.

PR Close #36944
---
 packages/bazel/src/ng_package/packager.ts | 35 +++++++++++++++++++----
 1 file changed, 30 insertions(+), 5 deletions(-)

diff --git a/packages/bazel/src/ng_package/packager.ts b/packages/bazel/src/ng_package/packager.ts
index 6ef447ac88..f72fa1b5eb 100644
--- a/packages/bazel/src/ng_package/packager.ts
+++ b/packages/bazel/src/ng_package/packager.ts
@@ -102,6 +102,13 @@ function main(args: string[]): number {
   const modulesManifest = JSON.parse(modulesManifestArg);
   const dtsBundles: string[] = dtsBundleArg.split(',').filter(s => !!s);
 
+  /**
+   * List of known `package.json` fields which provide information about
+   * supported package formats and their associated entry paths.
+   */
+  const knownFormatPackageJsonFields =
+      ['main', 'fesm2015', 'esm2015', 'typings', 'module', 'es2015'];
+
   if (readmeMd) {
     copyFile(readmeMd, out);
   }
@@ -324,11 +331,10 @@ function main(args: string[]): number {
     const packageName = parsedPackage['name'];
     const moduleData = modulesManifest[packageName];
 
-    // We don't want to modify the "package.json" if we guessed the entry-point
-    // paths and there is a custom "package.json" for that package already. Module
-    // data will be only undefined if the package name comes from a non-generated
-    // "package.json". In that case we want to leave the file untouched as well.
-    if (!moduleData || moduleData.guessedPaths && !isGeneratedPackageJson) {
+    // If a package json file has been discovered that does not match any
+    // module in the manifest, we report a warning as most likely the target
+    // is configured incorrectly (e.g. missing `module_name` attribute).
+    if (!moduleData) {
       // Ideally we should throw here, as we got an entry point that doesn't
       // have flat module metadata / bundle index, so it may have been an
       // ng_module that's missing a module_name attribute.
@@ -342,6 +348,19 @@ function main(args: string[]): number {
       return JSON.stringify(parsedPackage, null, 2);
     }
 
+    // If we guessed the index paths for a module, and it contains an explicit `package.json`
+    // file that already sets format properties, we skip automatic insertion of format
+    // properties but report a warning in case properties have been set by accident.
+    if (moduleData.guessedPaths && !isGeneratedPackageJson &&
+        hasExplicitFormatProperties(parsedPackage)) {
+      console.error('WARNING: `package.json` explicitly sets format properties (like `main`).');
+      console.error(
+          '    Skipping automatic insertion of format properties as explicit ' +
+          'format properties are set.');
+      console.error('    Ignore this warning if explicit properties are set intentionally.');
+      return JSON.stringify(parsedPackage, null, 2);
+    }
+
     // Derive the paths to the files from the hard-coded names we gave them.
     // TODO(alexeagle): it would be better to transfer this information from the place
     // where we created the filenames, via the modulesManifestArg
@@ -377,6 +396,12 @@ function main(args: string[]): number {
     return [relativePath, dir, basename + '.js'].join('/');
   }
 
+  /** Whether the package explicitly sets any of the format properties (like `main`). */
+  function hasExplicitFormatProperties(parsedPackage: {[key: string]: string}): boolean {
+    return Object.keys(parsedPackage)
+        .some(propertyName => knownFormatPackageJsonFields.includes(propertyName));
+  }
+
   /** Creates metadata re-export file for a secondary entry-point. */
   function createMetadataReexportFile(
       entryPointName: string, metadataFile: string, packageName: string) {