From b6078f588786329a4ced8db895c6bc21a6bd4b06 Mon Sep 17 00:00:00 2001 From: Chuck Jazdzewski Date: Wed, 14 Dec 2016 15:28:51 -0800 Subject: [PATCH] fix(compiler): update to metadata version 3 (#13464) This change retracts support for metadata version 2. The collector used to produce version 2 metadata was incomplete and can cause the AOT compiler to fail to resolve symbols or produce other spurious errors. All libraries compiled and published with 2.3.0 ngc will need to be recompiled and updated with this change. --- .../compiler-cli/src/compiler_host.ts | 18 +++---- .../compiler-cli/test/aot_host_spec.ts | 4 +- .../compiler/src/aot/static_reflector.ts | 10 ++-- .../test/aot/static_reflector_spec.ts | 51 +++++++++++-------- tools/@angular/tsc-wrapped/src/collector.ts | 10 +++- .../@angular/tsc-wrapped/src/compiler_host.ts | 18 ++----- tools/@angular/tsc-wrapped/src/schema.ts | 2 +- .../tsc-wrapped/test/collector.spec.ts | 12 ++--- 8 files changed, 66 insertions(+), 59 deletions(-) diff --git a/modules/@angular/compiler-cli/src/compiler_host.ts b/modules/@angular/compiler-cli/src/compiler_host.ts index 3360d1d28f..8d2ab99e06 100644 --- a/modules/@angular/compiler-cli/src/compiler_host.ts +++ b/modules/@angular/compiler-cli/src/compiler_host.ts @@ -177,31 +177,31 @@ export class CompilerHost implements AotCompilerHost { (Array.isArray(metadataOrMetadatas) ? metadataOrMetadatas : [metadataOrMetadatas]) : []; const v1Metadata = metadatas.find((m: any) => m['version'] === 1); - let v2Metadata = metadatas.find((m: any) => m['version'] === 2); - if (!v2Metadata && v1Metadata) { - // patch up v1 to v2 by merging the metadata with metadata collected from the d.ts file + let v3Metadata = metadatas.find((m: any) => m['version'] === 3); + if (!v3Metadata && v1Metadata) { + // patch up v1 to v3 by merging the metadata with metadata collected from the d.ts file // as the only difference between the versions is whether all exports are contained in // the metadata and the `extends` clause. - v2Metadata = {'__symbolic': 'module', 'version': 2, 'metadata': {}}; + v3Metadata = {'__symbolic': 'module', 'version': 3, 'metadata': {}}; if (v1Metadata.exports) { - v2Metadata.exports = v1Metadata.exports; + v3Metadata.exports = v1Metadata.exports; } for (let prop in v1Metadata.metadata) { - v2Metadata.metadata[prop] = v1Metadata.metadata[prop]; + v3Metadata.metadata[prop] = v1Metadata.metadata[prop]; } const exports = this.metadataCollector.getMetadata(this.getSourceFile(dtsFilePath)); if (exports) { for (let prop in exports.metadata) { - if (!v2Metadata.metadata[prop]) { - v2Metadata.metadata[prop] = exports.metadata[prop]; + if (!v3Metadata.metadata[prop]) { + v3Metadata.metadata[prop] = exports.metadata[prop]; } } if (exports.exports) { v2Metadata.exports = exports.exports; } } - metadatas.push(v2Metadata); + metadatas.push(v3Metadata); } this.resolverCache.set(filePath, metadatas); return metadatas; diff --git a/modules/@angular/compiler-cli/test/aot_host_spec.ts b/modules/@angular/compiler-cli/test/aot_host_spec.ts index 88334085d0..1134c9111c 100644 --- a/modules/@angular/compiler-cli/test/aot_host_spec.ts +++ b/modules/@angular/compiler-cli/test/aot_host_spec.ts @@ -166,11 +166,11 @@ describe('CompilerHost', () => { expect(hostNestedGenDir.getMetadataFor('node_modules/@angular/missing.d.ts')).toBeUndefined(); }); - it('should add missing v2 metadata from v1 metadata and .d.ts files', () => { + it('should add missing v3 metadata from v1 metadata and .d.ts files', () => { expect(hostNestedGenDir.getMetadataFor('metadata_versions/v1.d.ts')).toEqual([ {__symbolic: 'module', version: 1, metadata: {foo: {__symbolic: 'class'}}}, { __symbolic: 'module', - version: 2, + version: 3, metadata: { foo: {__symbolic: 'class'}, Bar: {__symbolic: 'class', members: {ngOnInit: [{__symbolic: 'method'}]}}, diff --git a/modules/@angular/compiler/src/aot/static_reflector.ts b/modules/@angular/compiler/src/aot/static_reflector.ts index 1b8d2dc070..8141f295a4 100644 --- a/modules/@angular/compiler/src/aot/static_reflector.ts +++ b/modules/@angular/compiler/src/aot/static_reflector.ts @@ -10,7 +10,7 @@ import {Attribute, Component, ContentChild, ContentChildren, Directive, Host, Ho import {ReflectorReader} from '../private_import_core'; import {StaticSymbol} from './static_symbol'; -const SUPPORTED_SCHEMA_VERSION = 2; +const SUPPORTED_SCHEMA_VERSION = 3; const ANGULAR_IMPORT_LOCATIONS = { coreDecorators: '@angular/core/src/metadata', diDecorators: '@angular/core/src/di/metadata', @@ -754,10 +754,10 @@ export class StaticReflector implements ReflectorReader { {__symbolic: 'module', version: SUPPORTED_SCHEMA_VERSION, module: module, metadata: {}}; } if (moduleMetadata['version'] != SUPPORTED_SCHEMA_VERSION) { - this.reportError( - new Error( - `Metadata version mismatch for module ${module}, found version ${moduleMetadata['version']}, expected ${SUPPORTED_SCHEMA_VERSION}`), - null); + const errorMessage = moduleMetadata['version'] == 2 ? + `Unsupported metadata version ${moduleMetadata['version']} for module ${module}. This module should be compiled with a newer version of ngc` : + `Metadata version mismatch for module ${module}, found version ${moduleMetadata['version']}, expected ${SUPPORTED_SCHEMA_VERSION}`; + this.reportError(new Error(errorMessage), null); } this.metadataCache.set(module, moduleMetadata); } diff --git a/modules/@angular/compiler/test/aot/static_reflector_spec.ts b/modules/@angular/compiler/test/aot/static_reflector_spec.ts index d38d5af3da..ebd09116f0 100644 --- a/modules/@angular/compiler/test/aot/static_reflector_spec.ts +++ b/modules/@angular/compiler/test/aot/static_reflector_spec.ts @@ -80,7 +80,13 @@ describe('StaticReflector', () => { it('should throw an exception for unsupported metadata versions', () => { expect(() => reflector.findDeclaration('src/version-error', 'e')) .toThrow(new Error( - 'Metadata version mismatch for module /tmp/src/version-error.d.ts, found version 100, expected 2')); + 'Metadata version mismatch for module /tmp/src/version-error.d.ts, found version 100, expected 3')); + }); + + it('should throw an exception for version 2 metadata', () => { + expect(() => reflector.findDeclaration('src/version-2-error', 'e')) + .toThrowError( + 'Unsupported metadata version 2 for module /tmp/src/version-2-error.d.ts. This module should be compiled with a newer version of ngc'); }); it('should get and empty annotation list for an unknown class', () => { @@ -384,7 +390,7 @@ describe('StaticReflector', () => { const metadata = reflector.getModuleMetadata('/tmp/src/custom-decorator-reference.ts'); expect(metadata).toEqual({ __symbolic: 'module', - version: 2, + version: 3, metadata: { Foo: { __symbolic: 'class', @@ -775,7 +781,7 @@ export class MockStaticReflectorHost implements StaticReflectorHost { const DEFAULT_TEST_DATA: {[key: string]: any} = { '/tmp/@angular/common/src/forms-deprecated/directives.d.ts': [{ '__symbolic': 'module', - 'version': 2, + 'version': 3, 'metadata': { 'FORM_DIRECTIVES': [ { @@ -788,7 +794,7 @@ const DEFAULT_TEST_DATA: {[key: string]: any} = { }], '/tmp/@angular/common/src/directives/ng_for.d.ts': { '__symbolic': 'module', - 'version': 2, + 'version': 3, 'metadata': { 'NgFor': { '__symbolic': 'class', @@ -841,16 +847,16 @@ const DEFAULT_TEST_DATA: {[key: string]: any} = { } }, '/tmp/@angular/core/src/linker/view_container_ref.d.ts': - {version: 2, 'metadata': {'ViewContainerRef': {'__symbolic': 'class'}}}, + {version: 3, 'metadata': {'ViewContainerRef': {'__symbolic': 'class'}}}, '/tmp/@angular/core/src/linker/template_ref.d.ts': - {version: 2, 'module': './template_ref', 'metadata': {'TemplateRef': {'__symbolic': 'class'}}}, + {version: 3, 'module': './template_ref', 'metadata': {'TemplateRef': {'__symbolic': 'class'}}}, '/tmp/@angular/core/src/change_detection/differs/iterable_differs.d.ts': - {version: 2, 'metadata': {'IterableDiffers': {'__symbolic': 'class'}}}, + {version: 3, 'metadata': {'IterableDiffers': {'__symbolic': 'class'}}}, '/tmp/@angular/core/src/change_detection/change_detector_ref.d.ts': - {version: 2, 'metadata': {'ChangeDetectorRef': {'__symbolic': 'class'}}}, + {version: 3, 'metadata': {'ChangeDetectorRef': {'__symbolic': 'class'}}}, '/tmp/src/app/hero-detail.component.d.ts': { '__symbolic': 'module', - 'version': 2, + 'version': 3, 'metadata': { 'HeroDetailComponent': { '__symbolic': 'class', @@ -1001,11 +1007,12 @@ const DEFAULT_TEST_DATA: {[key: string]: any} = { } } }, - '/src/extern.d.ts': {'__symbolic': 'module', 'version': 2, metadata: {s: 's'}}, + '/src/extern.d.ts': {'__symbolic': 'module', 'version': 3, metadata: {s: 's'}}, '/tmp/src/version-error.d.ts': {'__symbolic': 'module', 'version': 100, metadata: {e: 's'}}, + '/tmp/src/version-2-error.d.ts': {'__symbolic': 'module', 'version': 2, metadata: {e: 's'}}, '/tmp/src/error-reporting.d.ts': { __symbolic: 'module', - version: 2, + version: 3, metadata: { SomeClass: { __symbolic: 'class', @@ -1035,7 +1042,7 @@ const DEFAULT_TEST_DATA: {[key: string]: any} = { }, '/tmp/src/error-references.d.ts': { __symbolic: 'module', - version: 2, + version: 3, metadata: { Link1: { __symbolic: 'reference', @@ -1057,7 +1064,7 @@ const DEFAULT_TEST_DATA: {[key: string]: any} = { }, '/tmp/src/function-declaration.d.ts': { __symbolic: 'module', - version: 2, + version: 3, metadata: { one: { __symbolic: 'function', @@ -1086,7 +1093,7 @@ const DEFAULT_TEST_DATA: {[key: string]: any} = { }, '/tmp/src/function-reference.ts': { __symbolic: 'module', - version: 2, + version: 3, metadata: { one: { __symbolic: 'call', @@ -1128,7 +1135,7 @@ const DEFAULT_TEST_DATA: {[key: string]: any} = { }, '/tmp/src/function-recursive.d.ts': { __symbolic: 'modules', - version: 2, + version: 3, metadata: { recursive: { __symbolic: 'function', @@ -1188,7 +1195,7 @@ const DEFAULT_TEST_DATA: {[key: string]: any} = { }, '/tmp/src/spread.ts': { __symbolic: 'module', - version: 2, + version: 3, metadata: { spread: [0, {__symbolic: 'spread', expression: [1, 2, 3, 4]}, 5] } @@ -1338,7 +1345,7 @@ const DEFAULT_TEST_DATA: {[key: string]: any} = { `, '/tmp/src/reexport/reexport.d.ts': { __symbolic: 'module', - version: 2, + version: 3, metadata: {}, exports: [ {from: './src/origin1', export: ['One', 'Two', {name: 'Three', as: 'Four'}]}, @@ -1347,7 +1354,7 @@ const DEFAULT_TEST_DATA: {[key: string]: any} = { }, '/tmp/src/reexport/src/origin1.d.ts': { __symbolic: 'module', - version: 2, + version: 3, metadata: { One: {__symbolic: 'class'}, Two: {__symbolic: 'class'}, @@ -1356,26 +1363,26 @@ const DEFAULT_TEST_DATA: {[key: string]: any} = { }, '/tmp/src/reexport/src/origin5.d.ts': { __symbolic: 'module', - version: 2, + version: 3, metadata: { Five: {__symbolic: 'class'}, }, }, '/tmp/src/reexport/src/origin30.d.ts': { __symbolic: 'module', - version: 2, + version: 3, metadata: { Thirty: {__symbolic: 'class'}, }, }, '/tmp/src/reexport/src/originNone.d.ts': { __symbolic: 'module', - version: 2, + version: 3, metadata: {}, }, '/tmp/src/reexport/src/reexport2.d.ts': { __symbolic: 'module', - version: 2, + version: 3, metadata: {}, exports: [{from: './originNone'}, {from: './origin30'}] } diff --git a/tools/@angular/tsc-wrapped/src/collector.ts b/tools/@angular/tsc-wrapped/src/collector.ts index 3b5c5d2409..543f9b583e 100644 --- a/tools/@angular/tsc-wrapped/src/collector.ts +++ b/tools/@angular/tsc-wrapped/src/collector.ts @@ -17,6 +17,11 @@ import {Symbols} from './symbols'; * A set of collector options to use when collecting metadata. */ export class CollectorOptions { + /** + * Version of the metadata to collect. + */ + version?: number; + /** * Collect a hidden field "$quoted$" in objects literals that record when the key was quoted in * the source. @@ -430,7 +435,10 @@ export class MetadataCollector { else if (strict) { validateMetadata(sourceFile, nodeMap, metadata); } - const result: ModuleMetadata = {__symbolic: 'module', version: VERSION, metadata}; + const result: ModuleMetadata = { + __symbolic: 'module', + version: this.options.version || VERSION, metadata + }; if (exports) result.exports = exports; return result; } diff --git a/tools/@angular/tsc-wrapped/src/compiler_host.ts b/tools/@angular/tsc-wrapped/src/compiler_host.ts index 2fef207873..519dcd8b6e 100644 --- a/tools/@angular/tsc-wrapped/src/compiler_host.ts +++ b/tools/@angular/tsc-wrapped/src/compiler_host.ts @@ -110,7 +110,8 @@ export class TsickleCompilerHost extends DelegatingHost { const IGNORED_FILES = /\.ngfactory\.js$|\.ngstyle\.js$/; export class MetadataWriterHost extends DelegatingHost { - private metadataCollector = new MetadataCollector(); + private metadataCollector = new MetadataCollector({quotedNames: true}); + private metadataCollector1 = new MetadataCollector({version: 1}); constructor(delegate: ts.CompilerHost, private ngOptions: NgOptions) { super(delegate); } private writeMetadata(emitFilePath: string, sourceFile: ts.SourceFile) { @@ -120,18 +121,9 @@ export class MetadataWriterHost extends DelegatingHost { const path = emitFilePath.replace(/*DTS*/ /\.js$/, '.metadata.json'); const metadata = this.metadataCollector.getMetadata(sourceFile, !!this.ngOptions.strictMetadataEmit); - const metadatas: ModuleMetadata[] = [metadata]; - if (metadata && metadata.metadata) { - if (metadata.version === 2) { - // Also emit a version 1 so that older clients can consume new metadata files as well. - // We can write the same data as version 2 is a strict super set. - metadatas.push({ - __symbolic: metadata.__symbolic, - exports: metadata.exports, - metadata: metadata.metadata, - version: 1 - }); - } + const metadata1 = this.metadataCollector1.getMetadata(sourceFile, false); + const metadatas: ModuleMetadata[] = [metadata, metadata1].filter(e => !!e); + if (metadatas.length) { const metadataText = JSON.stringify(metadatas); writeFileSync(path, metadataText, {encoding: 'utf-8'}); } diff --git a/tools/@angular/tsc-wrapped/src/schema.ts b/tools/@angular/tsc-wrapped/src/schema.ts index b136e13179..05c2e48ac7 100644 --- a/tools/@angular/tsc-wrapped/src/schema.ts +++ b/tools/@angular/tsc-wrapped/src/schema.ts @@ -15,7 +15,7 @@ // semantics of the file in an array. For example, when generating a version 2 file, if version 1 // can accurately represent the metadata, generate both version 1 and version 2 in an array. -export const VERSION = 2; +export const VERSION = 3; export type MetadataEntry = ClassMetadata | FunctionMetadata | MetadataValue; diff --git a/tools/@angular/tsc-wrapped/test/collector.spec.ts b/tools/@angular/tsc-wrapped/test/collector.spec.ts index 1b8fe9ac24..b020579026 100644 --- a/tools/@angular/tsc-wrapped/test/collector.spec.ts +++ b/tools/@angular/tsc-wrapped/test/collector.spec.ts @@ -67,7 +67,7 @@ describe('Collector', () => { const metadata = collector.getMetadata(sourceFile); expect(metadata).toEqual({ __symbolic: 'module', - version: 2, + version: 3, metadata: { HeroDetailComponent: { __symbolic: 'class', @@ -108,7 +108,7 @@ describe('Collector', () => { const metadata = collector.getMetadata(sourceFile); expect(metadata).toEqual({ __symbolic: 'module', - version: 2, + version: 3, metadata: { AppComponent: { __symbolic: 'class', @@ -162,7 +162,7 @@ describe('Collector', () => { const metadata = collector.getMetadata(sourceFile); expect(metadata).toEqual({ __symbolic: 'module', - version: 2, + version: 3, metadata: { HEROES: [ {'id': 11, 'name': 'Mr. Nice', '$quoted$': ['id', 'name']}, @@ -241,7 +241,7 @@ describe('Collector', () => { const metadata = collector.getMetadata(unsupported1); expect(metadata).toEqual({ __symbolic: 'module', - version: 2, + version: 3, metadata: { a: {__symbolic: 'error', message: 'Destructuring not supported', line: 1, character: 16}, b: {__symbolic: 'error', message: 'Destructuring not supported', line: 1, character: 19}, @@ -283,7 +283,7 @@ describe('Collector', () => { const metadata = collector.getMetadata(sourceFile); expect(metadata).toEqual({ __symbolic: 'module', - version: 2, + version: 3, metadata: { SimpleClass: {__symbolic: 'class'}, AbstractClass: {__symbolic: 'class'}, @@ -297,7 +297,7 @@ describe('Collector', () => { const metadata = collector.getMetadata(exportedFunctions); expect(metadata).toEqual({ __symbolic: 'module', - version: 2, + version: 3, metadata: { one: { __symbolic: 'function',