fix(compiler-cli): incorrect bundled metadata for static class member call expressions (#28762)

Currently if developers use call expressions in their static
class members ([like we do in Angular](https://github.com/angular/angular/blob/master/packages/core/src/change_detection/differs/keyvalue_differs.ts#L121)),
the metadata that is generated for flat modules is invalid. This
is because the metadata bundler logic currently does not handle
call expressions in static class members and the symbol references
are not rewritten to avoid relative paths in the bundle.

Static class members using a call expression are not relevant for
the ViewEngine AOT compilation, but it is problematic that the
bundled metadata references modules using their original relative
path. This means that the bundled metadata is no longer encapsulated
and depends on other emitted files to be emitted in the proper place.

These incorrect relative paths can now cause issues where NGC
looks for the referenced symbols in the incorrect path. e.g.

```
src/
 | lib/
    | index.ts -> References the call expression using `../../di`
```

Now the metadata looks like that:

```
node_modules/
  | @angular/
  -- | core/
  -- -- | core.metadata.json -> Says that the call expr. is in `../../di`.
  | di/
```

Now if NGC tries to use the metadata files and create the summary files,
NGC resolves the call expression to the `node_modules/di` module. Since
the "unexpected" module does not contain the desired symbol, NGC will
error out.

We should fix this by ensuring that we don't ship corrupted metadata
to NPM which contains relative references that can cause such
failures (other imports can be affected as well; it depends on what
modules the developer has installed and how we import our call
expressions).

Fixes #28741.

PR Close #28762

Author: devversion

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

@@ -11,7 +11,7 @@ import * as ts from 'typescript';
 import {MetadataCache} from '../transformers/metadata_cache';
 
 import {MetadataCollector} from './collector';
-import {ClassMetadata, ConstructorMetadata, FunctionMetadata, METADATA_VERSION, MemberMetadata, MetadataEntry, MetadataError, MetadataImportedSymbolReferenceExpression, MetadataMap, MetadataObject, MetadataSymbolicExpression, MetadataSymbolicReferenceExpression, MetadataValue, MethodMetadata, ModuleExportMetadata, ModuleMetadata, isClassMetadata, isConstructorMetadata, isFunctionMetadata, isInterfaceMetadata, isMetadataError, isMetadataGlobalReferenceExpression, isMetadataImportedSymbolReferenceExpression, isMetadataModuleReferenceExpression, isMetadataSymbolicExpression, isMethodMetadata} from './schema';
+import {ClassMetadata, ConstructorMetadata, FunctionMetadata, METADATA_VERSION, MemberMetadata, MetadataEntry, MetadataError, MetadataMap, MetadataObject, MetadataSymbolicExpression, MetadataSymbolicReferenceExpression, MetadataValue, MethodMetadata, ModuleExportMetadata, ModuleMetadata, isClassMetadata, isConstructorMetadata, isFunctionMetadata, isInterfaceMetadata, isMetadataError, isMetadataGlobalReferenceExpression, isMetadataImportedSymbolReferenceExpression, isMetadataModuleReferenceExpression, isMetadataSymbolicCallExpression, isMetadataSymbolicExpression, isMethodMetadata} from './schema';
 
 
 
@@ -412,7 +412,17 @@ export class MetadataBundler {
     let result: StaticsMetadata = {};
     for (const key in statics) {
       const value = statics[key];
-      result[key] = isFunctionMetadata(value) ? this.convertFunction(moduleName, value) : value;
+
+      if (isFunctionMetadata(value)) {
+        result[key] = this.convertFunction(moduleName, value);
+      } else if (isMetadataSymbolicCallExpression(value)) {
+        // Class members can also contain static members that call a function with module
+        // references. e.g. "static ngInjectableDef = defineInjectable(..)". We also need to
+        // convert these module references because otherwise these resolve to non-existent files.
+        result[key] = this.convertValue(moduleName, value);
+      } else {
+        result[key] = value;
+      }
     }
     return result;
   }

packages/compiler-cli/test/metadata/bundler_spec.ts

@@ -218,6 +218,59 @@ describe('metadata bundler', () => {
     ]);
   });
 
+  it('should rewrite call expression references for static class members', () => {
+    const host = new MockStringBundlerHost('/', {
+      'lib': {
+        'index.ts': `export * from './deep/index';`,
+        'shared.ts': `
+          export function sharedFn() {
+            return {foo: true};
+          }`,
+        'deep': {
+          'index.ts': `
+            import {sharedFn} from '../shared';
+          
+            export class MyClass {
+              static ngInjectableDef = sharedFn(); 
+            }
+          `,
+        }
+      }
+    });
+    const bundler = new MetadataBundler('/lib/index', undefined, host);
+    const bundledMetadata = bundler.getMetadataBundle().metadata;
+    const deepIndexMetadata = host.getMetadataFor('/lib/deep/index') !;
+
+    // The unbundled metadata should reference symbols using the relative module path.
+    expect(deepIndexMetadata.metadata['MyClass']).toEqual(jasmine.objectContaining({
+      statics: {
+        ngInjectableDef: {
+          __symbolic: 'call',
+          expression: {
+            __symbolic: 'reference',
+            name: 'sharedFn',
+            module: '../shared',
+          }
+        }
+      }
+    }));
+
+    // For the bundled metadata, the "sharedFn" symbol should not be referenced using the
+    // relative module path (like for unbundled), because the metadata bundle can be stored
+    // anywhere and it's not guaranteed that the relatively referenced files are present.
+    expect(bundledMetadata.metadata['MyClass']).toEqual(jasmine.objectContaining({
+      statics: {
+        ngInjectableDef: {
+          __symbolic: 'call',
+          expression: {
+            __symbolic: 'reference',
+            name: 'ɵa',
+          }
+        }
+      }
+    }));
+  });
+
   it('should be able to bundle an oddly constructed library', () => {
     const host = new MockStringBundlerHost('/', {
       'lib': {