Angular components bazel package output

Overview of issues that came up and needed investigation/workarounds:

Description	Workaround?
ng_package rule that implements Angular Package Format breaks for secondary entry-points with factories. See issue.	✅ (needs proposal)
ng_package rule not working on windows with many rollup globals. See issue	✅ (needs fix)
ng_module rule does not allow specification of flat module entry-point. See proposal PR to fix this	✅ (proposal; needs review)
ng_package rule does not support ts_library targets for secondary entry-points. See PR to fix this	✅ (fix landed)
ng_module rules generate invalid imports for injectables that rely on symbols from flat module bundles. More details below	✅ (needs fix). Very difficult to land upstream. See explanation of the problem to understand why.
ng_module rules generate invalid imports forinjectables that leak Bazel-specific workspace paths. More details below	❌ (fix in progress; not clear whether it's possible to land)

Invalid imports blocker

The ng_module bazel rule does not include metadata files when building
a package. For example, if we build material/snack-bar, the metadata
files of cdk/overlay are not included.

Just the summary files of the cdk/overlay are available. This means that
the compilation misses information about the flat module bundle of
cdk/overlay and uses the summaries to resolve exported symbols.

Since summaries are generated on a per-file basis, there is no
concept of summaries for the dynamically generated flat module
bundle. This means that symbols across different entry-points
will be resolved to their original source file location.

This causes NGC to generate imports to deep files rather than using
the flat module entry-point (which is usually index.js). e.g.

var i1 = require("@angular/cdk/overlay/overlay");
var i2 = require("@angular/cdk/a11y/live-announcer/live-announcer");
var i3 = require("@angular/cdk/layout/breakpoints-observer");
var i4 = require("@angular/material/snack-bar/snack-bar-module");

This is problematic because as per implementation of the Angular
package format, references to other entry-points are only
possible if done through the actual entry-point (no deep imports).

This is necessary to support the individual package.json fields
for deeply-referenced entry-points (e.g. es2015, main, fesm2015).

Note: here is an isolated reproduction that demonstrates
the issue: https://github.com/devversion/bazel-ngc-wrapped-invalid-imports

Why does this not surface in the current Gulp setup?

This issue does not happen within the current gulp setup because
our subsequent compilations have access to metadata files of
previous entry-points.

e.g. cdk/overlay is compiled with a flat module bundle and
stored in dist/packages/cdk/overlay/. Now compilations for
material/snack-bar can just have a path mapping to dist/packages/cdk/overlay,
and NGC will be able to properly generate imports through
the flat module bundle entry-point (as it knows that everything
is exported in the entry-point due to the flat module metadata)

How can we fix it?

It looks like ngc-wrapped (which is the wrapper for the ng_module rule), overwrites
the fileExists method of the host that is passed to NGC.

bazelHost.fileExists = (fileName: string) => {
    // For ngsummary.json files (which match this regex), the "fileExists"
    // call will be delegated to the real TypeScript host. So it will check
    // on the disk.
    if (NGC_ASSETS.test(fileName)) {
      return tsHost.fileExists(fileName);
    }
    
    // For other files which aren't `ngsummary.json` files, the
    // `@bazel/typescript` compiler host will be used. **NOTE**
    // that this host does not check the *real file system*.. it
    // just checks if the file is part of the Bazel action inputs.
    // Metadata files are not added as inputs of the compilation!
    return origBazelHostFileExist.call(bazelHost, fileName);
};

https://github.com/angular/angular/blob/2b64031ddc1bb90b1e7a7da04937763c7a49b567/packages/bazel/src/ngc-wrapped/index.ts#L257-L262

Modifying the NGC_ASSETS regular expression in ngc-wrapped to allow .metadata.json
files to be take into account, fixes the issue. Though it unveils another issue for
the Bazel ng_module compilations.

What's the issue if we allow metadata files in Bazel?

Disclaimer: There might be more reasons why metadata files are
not allowed in the Bazel compilations, but this is the one that appears
in the components repository if we allow metadata files.

Let's consider we fixed the metadata reading issues in ngc-wrapped (as of the
section above). Now that symbols are resolved through metadata (and not through summaries), the StaticSymbolResolver handles the symbol resolution.

Within the compiler, there seems to be logic to discard metadata of class symbols
which are from previous compilation units:

if (this.summaryResolver.isLibraryFile(sourceSymbol.filePath) && !isTsFile && metadata &&
    metadata['__symbolic'] === 'class') {
  const transformedMeta = {__symbolic: 'class', arity: metadata.arity};
  return new ResolvedStaticSymbol(sourceSymbol, transformedMeta);
}

https://github.com/angular/angular/blob/master/packages/compiler/src/aot/static_symbol_resolver.ts#L374-L384

We don't have this issue in the angular/components repository with our Gulp setup because
it seems that the issue depends on the generateCodeForLibraries flag.

The ng_module rule always it sets generateCodeForLibraries to false, while in vanilla
ngc the flag is enabled by default. If the flag is set to false, the isLibraryFile
check (see snippet above) will be always true for files from other compilation units and
the metadata will be discarded. Causing the compilations to fail with exceptions like:

Unexpected value 'BidiModule in C:/users/paul/_bazel_paul/kn4tsvyh/execroot/angular_material/bazel-out/x64_windows-fastbuild/bin/src/cdk/bidi/index.d.ts' imported by the module 'ScrollingModule in C:/users/paul/_bazel_paul/kn4tsvyh/execroot/angular_material/src/cdk/scrolling/scro
lling-module.ts'. Please add a @NgModule annotation.

Here is a reproduction for that issue: https://github.com/devversion/bazel-ngc-wrapped-invalid-imports#how-to-reproduce-the-bazel-generatecodeforlibaries-issue

Paul Not setting generateCodeForLibaries to false would solve the issue, but I
guess the reason we explicitly set it to false is that otherwise compilations would
write genfiles to locations which are out of scope of the current package. Basically
it would break hermeticity of the ng_module rule. It feels wrong to me that
metadata resolution depends on the generateCodeForLibraries flag. The flag is
described to only prevent generation of genfiles for libraries (which makes sense
in bazel to keep hermeticity), but apparently it does more than that.

Second issue that came up with import generation

With the section above, we handled the issue of invalid imports being generated that should
actually resolve to flat module bundle entries.

A second issue with invalid imports comes from the fact that the ng_module rule always
uses absolute manifest paths for importing symbols that aren't known to the summary
resolver, or static symbol resolver. See the actual code for details.

This issue commonly surfaces when services inject other symbols from the same entry-point. Concrete example in the components focus-trap. The ng_module rule leaks Bazel specific paths into the release output. This is problematic, and the actual import should be relative since it's part of the same entry-point.

The intent was to fix this for the ng_module rule, by never using full manifest paths for generated imports within the compilation unit. See: angular#35841 for more details.