fix(bazel): ng_package should include private exports in fesms (#23054)

PR Close #23054

packages/bazel/src/BUILD.bazel

@@ -26,15 +26,3 @@ nodejs_binary(
     data = ["modify_tsconfig.js"],
     entry_point = "angular/packages/bazel/src/modify_tsconfig.js",
 )
-
-nodejs_binary(
-    name = "index_bundler",
-    data = [
-        # BEGIN-INTERNAL
-        # Only needed when compiling within the Angular repo.
-        # Users will get this dependency from node_modules.
-        "//packages/compiler-cli",
-        # END-INTERNAL
-    ],
-    entry_point = "@angular/compiler-cli/src/metadata/bundle_index_main.js",
-)

packages/bazel/src/ng_module.bzl

@@ -12,7 +12,6 @@ load(":rules_typescript.bzl",
     "compile_ts",
     "DEPS_ASPECTS",
     "ts_providers_dict_to_struct",
-    "json_marshal",
 )
 
 def _basename_of(ctx, file):
@@ -23,6 +22,36 @@ def _basename_of(ctx, file):
     ext_len = len(".html")
   return file.short_path[len(ctx.label.package) + 1:-ext_len]
 
+def _flat_module_out_file(ctx):
+  """Provide a default for the flat_module_out_file attribute.
+
+  We cannot use the default="" parameter of ctx.attr because the value is calculated
+  from other attributes (name)
+
+  Args:
+    ctx: skylark rule execution context
+
+  Returns:
+    a basename used for the flat module out (no extension)
+  """
+  if hasattr(ctx.attr, "flat_module_out_file") and ctx.attr.flat_module_out_file:
+    return ctx.attr.flat_module_out_file
+  return "%s_public_index" % ctx.label.name
+
+def _should_produce_flat_module_outs(ctx):
+  """Should we produce flat module outputs.
+
+  We only produce flat module outs when we expect the ng_module is meant to be published,
+  based on the presence of the module_name attribute.
+
+  Args:
+    ctx: skylark rule execution context
+
+  Returns:
+    true iff we should run the bundle_index_host to produce flat module metadata and bundle index
+  """
+  return bool(ctx.attr.module_name)
+
 # Calculate the expected output of the template compiler for every source in
 # in the library. Most of these will be produced as empty files but it is
 # unknown, without parsing, which will be empty.
@@ -31,6 +60,7 @@ def _expected_outs(ctx):
   closure_js_files = []
   declaration_files = []
   summary_files = []
+  metadata_files = []
 
   factory_basename_set = depset([_basename_of(ctx, src) for src in ctx.files.factories])
 
@@ -73,6 +103,17 @@ def _expected_outs(ctx):
     declaration_files += [ctx.new_file(ctx.bin_dir, basename + ext) for ext in declarations]
     summary_files += [ctx.new_file(ctx.bin_dir, basename + ext) for ext in summaries]
 
+  # We do this just when producing a flat module index for a publishable ng_module
+  if _should_produce_flat_module_outs(ctx):
+    flat_module_out = _flat_module_out_file(ctx)
+    devmode_js_files.append(ctx.actions.declare_file("%s.js" % flat_module_out))
+    closure_js_files.append(ctx.actions.declare_file("%s.closure.js" % flat_module_out))
+    bundle_index_typings = ctx.actions.declare_file("%s.d.ts" % flat_module_out)
+    declaration_files.append(bundle_index_typings)
+    metadata_files.append(ctx.actions.declare_file("%s.metadata.json" % flat_module_out))
+  else:
+    bundle_index_typings = None
+
   i18n_messages_files = [ctx.new_file(ctx.genfiles_dir, ctx.label.name + "_ngc_messages.xmb")]
 
   return struct(
@@ -80,6 +121,8 @@ def _expected_outs(ctx):
     devmode_js = devmode_js_files,
     declarations = declaration_files,
     summaries = summary_files,
+    metadata = metadata_files,
+    bundle_index_typings = bundle_index_typings,
     i18n_messages = i18n_messages_files,
   )
 
@@ -92,21 +135,29 @@ def _ngc_tsconfig(ctx, files, srcs, **kwargs):
 def _ngc_tsconfig_helper(ctx, files, srcs, enable_ivy, **kwargs):
   outs = _expected_outs(ctx)
   if "devmode_manifest" in kwargs:
-    expected_outs = outs.devmode_js + outs.declarations + outs.summaries
+    expected_outs = outs.devmode_js + outs.declarations + outs.summaries + outs.metadata
   else:
     expected_outs = outs.closure_js
 
+  angular_compiler_options = {
+      "enableResourceInlining": ctx.attr.inline_resources,
+      "generateCodeForLibraries": False,
+      "allowEmptyCodegenFiles": True,
+      "enableSummariesForJit": True,
+      "enableIvy": enable_ivy,
+      "fullTemplateTypeCheck": ctx.attr.type_check,
+      # FIXME: wrong place to de-dupe
+      "expectedOut": depset([o.path for o in expected_outs]).to_list()
+  }
+
+  if _should_produce_flat_module_outs(ctx):
+    angular_compiler_options["flatModuleId"] = ctx.attr.module_name
+    angular_compiler_options["flatModuleOutFile"] = _flat_module_out_file(ctx)
+    angular_compiler_options["flatModulePrivateSymbolPrefix"] = "_".join(
+        [ctx.workspace_name] + ctx.label.package.split("/") + [ctx.label.name, ""])
+
   return dict(tsc_wrapped_tsconfig(ctx, files, srcs, **kwargs), **{
-      "angularCompilerOptions": {
-          "enableResourceInlining": ctx.attr.inline_resources,
-          "generateCodeForLibraries": False,
-          "allowEmptyCodegenFiles": True,
-          "enableSummariesForJit": True,
-          "enableIvy": enable_ivy,
-          "fullTemplateTypeCheck": ctx.attr.type_check,
-          # FIXME: wrong place to de-dupe
-          "expectedOut": depset([o.path for o in expected_outs]).to_list()
-      }
+      "angularCompilerOptions": angular_compiler_options
   })
 
 def _collect_summaries_aspect_impl(target, ctx):
@@ -243,7 +294,7 @@ def _prodmode_compile_action(ctx, inputs, outputs, tsconfig_file, node_opts):
 
 def _devmode_compile_action(ctx, inputs, outputs, tsconfig_file, node_opts):
   outs = _expected_outs(ctx)
-  compile_action_outputs = outputs + outs.devmode_js + outs.declarations + outs.summaries
+  compile_action_outputs = outputs + outs.devmode_js + outs.declarations + outs.summaries + outs.metadata
   _compile_action(ctx, inputs, compile_action_outputs, None, tsconfig_file, node_opts)
 
 def _ts_expected_outs(ctx, label):
@@ -252,71 +303,6 @@ def _ts_expected_outs(ctx, label):
   _ignored = [label]
   return _expected_outs(ctx)
 
-def _write_bundle_index(ctx):
-  """ Provide an action that runs the bundle_index_main in ngc.
-
-  The action will only be executed when bundling with ng_package with a dep[] on this ng_module.
-
-  This creates:
-  - "flat module" metadata files
-  - "bundle index" js files with private symbol re-export, and their accompanying .d.ts files
-
-  Args:
-    ctx: the skylark rule execution context
-
-  Returns:
-    A struct indicating where the files were produced, which is passed through an ng_package rule to packager.ts
-  """
-  # Provide a default for the flat_module_out_file attribute.
-  # We cannot use the default="" parameter of ctx.attr because the value is calculated
-  # from other attributes (name)
-  flat_module_out_file = ctx.attr.flat_module_out_file if ctx.attr.flat_module_out_file else "%s_public_index" % ctx.label.name
-
-  tsconfig_file = ctx.actions.declare_file("%s.tsconfig.json" % flat_module_out_file)
-  metadata_file = ctx.actions.declare_file("%s.metadata.json" % flat_module_out_file)
-  typings_file = ctx.actions.declare_file("%s.d.ts" % flat_module_out_file)
-  index_file = ctx.actions.declare_file("%s.js" % flat_module_out_file)
-
-  tsconfig = dict(tsc_wrapped_tsconfig(ctx, ctx.files.srcs, ctx.files.srcs), **{
-    "angularCompilerOptions": {
-      "flatModuleOutFile": flat_module_out_file,
-      "flatModulePrivateSymbolPrefix": "_".join([ctx.workspace_name] + ctx.label.package.split("/") + [ctx.label.name, ""]),
-    },
-  })
-  if not ctx.attr.module_name:
-    fail("""Internal error: bundle index files are only produced for ng_module targets with a module_name.
-            Please file a bug at https://github.com/angular/angular/issues""")
-  tsconfig["angularCompilerOptions"]["flatModuleId"] = ctx.attr.module_name
-
-  entry_point = ctx.attr.entry_point if ctx.attr.entry_point else "index.ts"
-  # createBundleIndexHost in bundle_index_host.ts will throw if the "files" has more than one entry.
-  # We don't want to fail() here, however, because not all ng_module's will have the bundle index written.
-  # So we make the assumption that the index.ts file in the highest parent directory is the entry point.
-  index = None
-
-  for f in tsconfig["files"]:
-    if f.endswith("/" + entry_point):
-      if not index or len(f) < len(index):
-        index = f
-
-  if index:
-    tsconfig["files"] = [index]
-
-  ctx.actions.write(tsconfig_file, json_marshal(tsconfig))
-
-  ctx.action(
-      progress_message = "Producing metadata for bundle %s" % ctx.label.name,
-      executable = ctx.executable._index_bundler,
-      inputs = ctx.files.srcs + [tsconfig_file],
-      outputs = [metadata_file, typings_file, index_file],
-      arguments = ["-p", tsconfig_file.path],
-  )
-  return struct(
-      module_name = ctx.attr.module_name,
-      metadata_file = metadata_file,
-      typings_file = typings_file,
-      index_file = index_file)
-
 def ng_module_impl(ctx, ts_compile_actions, ivy = False):
   """Implementation function for the ng_module rule.
 
@@ -343,18 +329,20 @@ def ng_module_impl(ctx, ts_compile_actions, ivy = False):
 
   outs = _expected_outs(ctx)
   providers["angular"] = {
-    "summaries": _expected_outs(ctx).summaries
+    "summaries": outs.summaries
   }
   providers["ngc_messages"] = outs.i18n_messages
 
-  # Only produces the flattened "index bundle" metadata when requested by some other rule
-  # and only under Bazel
-  if hasattr(ctx.executable, "_index_bundler") and ctx.attr.module_name:
-    bundle_index_metadata = [_write_bundle_index(ctx)]
-  else:
-    bundle_index_metadata = []
+  if _should_produce_flat_module_outs(ctx):
+    if len(outs.metadata) > 1:
+      fail("expecting exactly one metadata output for " + str(ctx.label))
 
-  providers["angular"]["flat_module_metadata"] = depset(bundle_index_metadata)
+    providers["angular"]["flat_module_metadata"] = struct(
+      module_name = ctx.attr.module_name,
+      metadata_file = outs.metadata[0],
+      typings_file = outs.bundle_index_typings,
+      flat_module_out_file = _flat_module_out_file(ctx),
+    )
 
   return providers
 
@@ -422,11 +410,6 @@ NG_MODULE_RULE_ATTRS = dict(dict(COMMON_ATTRIBUTES, **NG_MODULE_ATTRIBUTES), **{
     # See the flatModuleOutFile documentation in
     # https://github.com/angular/angular/blob/master/packages/compiler-cli/src/transformers/api.ts
     "flat_module_out_file": attr.string(),
-
-    "_index_bundler": attr.label(
-        executable = True,
-        cfg = "host",
-        default = Label("//packages/bazel/src:index_bundler")),
 })
 
 ng_module = rule(

packages/bazel/src/ng_package/ng_package.bzl

@@ -122,28 +122,31 @@ def _ng_package_impl(ctx):
   # - ng_module rules in the deps (they have an "angular" provider)
   # - in this package or a subpackage
   # - those that have a module_name attribute (they produce flat module metadata)
-  entry_points = []
   flat_module_metadata = []
-  for dep in ctx.attr.deps:
-    if dep.label.package.startswith(ctx.label.package):
-      entry_points.append(dep.label.package[len(ctx.label.package) + 1:])
-      if hasattr(dep, "angular") and dep.angular.flat_module_metadata:
-        flat_module_metadata.append(dep.angular.flat_module_metadata)
+  deps_in_package = [d for d in ctx.attr.deps if d.label.package.startswith(ctx.label.package)]
+  for dep in deps_in_package:
+    # Intentionally evaluates to empty string for the main entry point
+    entry_point = dep.label.package[len(ctx.label.package) + 1:]
+    if hasattr(dep, "angular") and hasattr(dep.angular, "flat_module_metadata"):
+      flat_module_metadata.append(dep.angular.flat_module_metadata)
+      flat_module_out_file = dep.angular.flat_module_metadata.flat_module_out_file + ".js"
+    else:
+      # fallback to a reasonable default
+      flat_module_out_file = "index.js"
 
-  for entry_point in entry_points:
     es2015_entry_point = "/".join([p for p in [
         ctx.bin_dir.path,
         ctx.label.package,
         ctx.label.name + ".es6",
         ctx.label.package,
         entry_point,
-        "index.js",
+        flat_module_out_file,
     ] if p])
 
     es5_entry_point = "/".join([p for p in [
         ctx.label.package,
         entry_point,
-        "index.js",
+        flat_module_out_file,
     ] if p])
 
     if entry_point:
@@ -192,10 +195,10 @@ def _ng_package_impl(ctx):
   # Marshal the metadata into a JSON string so we can parse the data structure
   # in the TypeScript program easily.
   metadata_arg = {}
-  for m in depset(transitive = flat_module_metadata).to_list():
-    packager_inputs.extend([m.index_file, m.typings_file, m.metadata_file])
+  for m in flat_module_metadata:
+    packager_inputs.extend([m.metadata_file])
     metadata_arg[m.module_name] = {
-        "index": m.index_file.path,
+        "index": m.typings_file.path.replace(".d.ts", ".js"),
         "typings": m.typings_file.path,
         "metadata": m.metadata_file.path,
     }
@@ -226,7 +229,7 @@ def _ng_package_impl(ctx):
     packager_args.add("")
 
   ctx.actions.run(
-      progress_message = "Angular Packaging: building npm package for %s" % ctx.label.name,
+      progress_message = "Angular Packaging: building npm package %s" % str(ctx.label),
       mnemonic = "AngularPackage",
       inputs = packager_inputs,
       outputs = [npm_package_directory],

packages/bazel/src/ng_package/packager.ts

@@ -165,16 +165,11 @@ function main(args: string[]): number {
   // of the index JS file, which we need to amend the package.json.
   Object.keys(modulesManifest).forEach(moduleName => {
     const moduleFiles = modulesManifest[moduleName];
-    const indexContent = fs.readFileSync(moduleFiles['index'], 'utf-8');
     const relative = path.relative(binDir, moduleFiles['index']);
 
     moduleFiles['esm5_index'] = path.join(binDir, 'esm5', relative);
     moduleFiles['esm2015_index'] = path.join(binDir, 'esm2015', relative);
 
-    writeFileFromInputPath(moduleFiles['esm5_index'], indexContent);
-    writeFileFromInputPath(moduleFiles['esm2015_index'], indexContent);
-
-    copyFileFromInputPath(moduleFiles['typings']);
     copyFileFromInputPath(moduleFiles['metadata']);
   });
 

packages/bazel/src/ngc-wrapped/index.ts

@@ -180,8 +180,18 @@ export function compile({allowNonHermeticReads, allDepsCompiledWithBazel = true,
     return origBazelHostFileExist.call(bazelHost, fileName);
   };
   const origBazelHostShouldNameModule = bazelHost.shouldNameModule.bind(bazelHost);
-  bazelHost.shouldNameModule = (fileName: string) =>
-      origBazelHostShouldNameModule(fileName) || NGC_GEN_FILES.test(fileName);
+  bazelHost.shouldNameModule = (fileName: string) => {
+    // The bundle index file is synthesized in bundle_index_host so it's not in the
+    // compilationTargetSrc.
+    // However we still want to give it an AMD module name for devmode.
+    // We can't easily tell which file is the synthetic one, so we build up the path we expect
+    // it to have
+    // and compare against that.
+    if (fileName ===
+        path.join(compilerOpts.baseUrl, bazelOpts.package, compilerOpts.flatModuleOutFile + '.ts'))
+      return true;
+    return origBazelHostShouldNameModule(fileName) || NGC_GEN_FILES.test(fileName);
+  };
 
   const ngHost = ng.createCompilerHost({options: compilerOpts, tsHost: bazelHost});
 

packages/bazel/test/ng_package/core_package.spec.ts

@@ -107,6 +107,9 @@ describe('@angular/core ng_package', () => {
         expect(shx.cat('fesm2015/core.js'))
             .toMatch(/@license Angular v\d+\.\d+\.\d+(?!-PLACEHOLDER)/);
       });
+
+      it('should have been built from the generated bundle index',
+         () => { expect(shx.cat('fesm2015/core.js')).toMatch('export {.*makeParamDecorator'); });
     });
 
 
@@ -127,6 +130,9 @@ describe('@angular/core ng_package', () => {
         expect(shx.cat('fesm5/core.js')).not.toContain('function __extends');
         expect(shx.cat('fesm5/core.js')).toMatch('import {.*__extends');
       });
+
+      it('should have been built from the generated bundle index',
+         () => { expect(shx.cat('fesm5/core.js')).toMatch('export {.*makeParamDecorator'); });
     });
 
 

packages/compiler-cli/integrationtest/bazel/ng_module/extract_flat_module_index.bzl

@@ -9,8 +9,8 @@ def _extract_flat_module_index(ctx):
   files = []
   for dep in ctx.attr.deps:
     if hasattr(dep, "angular"):
-      for metadata in dep.angular.flat_module_metadata:
-        files.extend([metadata.metadata_file, metadata.typings_file])
+      metadata = dep.angular.flat_module_metadata
+      files.extend([metadata.metadata_file, metadata.typings_file])
   return [DefaultInfo(files = depset(files))]
 
 extract_flat_module_index = rule(

packages/compiler-cli/src/metadata/bundle_index_host.ts

@@ -37,7 +37,11 @@ function createSyntheticIndexHost<H extends ts.CompilerHost>(
   newHost.getSourceFile =
       (fileName: string, languageVersion: ts.ScriptTarget, onError?: (message: string) => void) => {
         if (path.normalize(fileName) == normalSyntheticIndexName) {
-          return ts.createSourceFile(fileName, indexContent, languageVersion, true);
+          const sf = ts.createSourceFile(fileName, indexContent, languageVersion, true);
+          if ((delegate as any).fileNameToModuleName) {
+            sf.moduleName = (delegate as any).fileNameToModuleName(fileName);
+          }
+          return sf;
         }
         return delegate.getSourceFile(fileName, languageVersion, onError);
       };
@@ -48,10 +52,10 @@ function createSyntheticIndexHost<H extends ts.CompilerHost>(
        sourceFiles: Readonly<ts.SourceFile>[]) => {
         delegate.writeFile(fileName, data, writeByteOrderMark, onError, sourceFiles);
         if (fileName.match(DTS) && sourceFiles && sourceFiles.length == 1 &&
-            path.normalize(sourceFiles[0].fileName) == normalSyntheticIndexName) {
+            path.normalize(sourceFiles[0].fileName) === normalSyntheticIndexName) {
           // If we are writing the synthetic index, write the metadata along side.
           const metadataName = fileName.replace(DTS, '.metadata.json');
-          fs.writeFileSync(metadataName, indexMetadata, {encoding: 'utf8'});
+          delegate.writeFile(metadataName, indexMetadata, writeByteOrderMark, onError, []);
         }
       };
   return newHost;
@@ -61,7 +65,20 @@ export function createBundleIndexHost<H extends ts.CompilerHost>(
     ngOptions: CompilerOptions, rootFiles: ReadonlyArray<string>,
     host: H): {host: H, indexName?: string, errors?: ts.Diagnostic[]} {
   const files = rootFiles.filter(f => !DTS.test(f));
-  if (files.length != 1) {
+  let indexFile: string|undefined;
+  if (files.length === 1) {
+    indexFile = files[0];
+  } else {
+    for (const f of files) {
+      // Assume the shortest file path called index.ts is the entry point
+      if (f.endsWith(path.sep + 'index.ts')) {
+        if (!indexFile || indexFile.length > f.length) {
+          indexFile = f;
+        }
+      }
+    }
+  }
+  if (!indexFile) {
     return {
       host,
       errors: [{
@@ -75,8 +92,8 @@ export function createBundleIndexHost<H extends ts.CompilerHost>(
       }]
     };
   }
-  const file = files[0];
-  const indexModule = file.replace(/\.ts$/, '');
+
+  const indexModule = indexFile.replace(/\.ts$/, '');
   const bundler = new MetadataBundler(
       indexModule, ngOptions.flatModuleId, new CompilerHostAdapter(host),
       ngOptions.flatModulePrivateSymbolPrefix);

packages/compiler-cli/src/metadata/bundle_index_main.ts

@@ -1,43 +0,0 @@
-#!/usr/bin/env node
-/**
- * @license
- * Copyright Google Inc. All Rights Reserved.
- *
- * 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
- */
-
-// Must be imported first, because Angular decorators throw on load.
-import 'reflect-metadata';
-
-import * as ts from 'typescript';
-import * as path from 'path';
-import {readCommandLineAndConfiguration} from '../main';
-import {createBundleIndexHost} from './bundle_index_host';
-import * as ng from '../transformers/entry_points';
-
-export function main(args: string[], consoleError: (s: string) => void = console.error): number {
-  const {options, rootNames} = readCommandLineAndConfiguration(args);
-  const host = ng.createCompilerHost({options});
-  const {host: bundleHost, indexName, errors} = createBundleIndexHost(options, rootNames, host);
-  if (!indexName) {
-    console.error('Did not find an index.ts in the top-level of the package.');
-    return 1;
-  }
-  // The index file is synthetic, so we have to add it to the program after parsing the tsconfig
-  rootNames.push(indexName);
-  const program = ts.createProgram(rootNames, options, bundleHost);
-  const indexSourceFile = program.getSourceFile(indexName);
-  if (!indexSourceFile) {
-    console.error(`${indexSourceFile} is not in the program. Please file a bug.`);
-    return 1;
-  }
-  program.emit(indexSourceFile);
-  return 0;
-}
-
-// CLI entry point
-if (require.main === module) {
-  const args = process.argv.slice(2);
-  process.exitCode = main(args);
-}

packages/compiler-cli/tsconfig-build.json

@@ -27,7 +27,6 @@
     "index.ts",
     "ngtools2.ts",
     "src/main.ts",
-    "src/metadata/bundle_index_main.ts",
     "src/extract_i18n.ts",
     "src/language_services.ts",
     "../../node_modules/@types/node/index.d.ts",