# 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](https://github.com/angular/angular/issues/32651). | ✅ (needs proposal)|
| `ng_package` rule not working on windows with many rollup globals. [See issue](https://github.com/angular/angular/issues/32603) |✅ (needs fix) |
| `ng_module` rule does not allow specification of flat module entry-point. [See proposal PR to fix this](https://github.com/angular/angular/pulls) | ✅ (proposal; needs review)|
| `ng_package` rule does not support `ts_library` targets for secondary entry-points. [See PR to fix this](https://github.com/angular/angular/pull/32610) | ✅ (fix landed) |
| `ng_module` rules generate invalid imports for `injectables` that rely on symbols from flat module bundles. [More details below](#invalid-imports-blocker) | ✅ (needs fix). Very difficult to land upstream. See explanation of the problem to understand why. |
| `ng_module` rules generate invalid imports for`injectables` 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

> [name=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](https://github.com/angular/angular/blob/master/packages/compiler/src/aot/compiler.ts#L702-L705).

This issue commonly surfaces when services inject other symbols from the same entry-point. [Concrete example in the components focus-trap](https://unpkg.com/browse/@angular/cdk@9.0.0/esm2015/a11y/focus-trap/focus-trap.js). 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](https://github.com/angular/angular/pull/35841) for more details.