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
This commit is contained in:
parent
692ddfcbb5
commit
4131715df5
|
@ -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;
|
||||
}
|
||||
|
|
|
@ -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': {
|
||||
|
|
Loading…
Reference in New Issue