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.
This commit is contained in:
Chuck Jazdzewski 2016-12-14 15:28:51 -08:00 committed by Victor Berchet
parent c65b4fa9dc
commit b6078f5887
8 changed files with 66 additions and 59 deletions

View File

@ -177,31 +177,31 @@ export class CompilerHost implements AotCompilerHost {
(Array.isArray(metadataOrMetadatas) ? metadataOrMetadatas : [metadataOrMetadatas]) : (Array.isArray(metadataOrMetadatas) ? metadataOrMetadatas : [metadataOrMetadatas]) :
[]; [];
const v1Metadata = metadatas.find((m: any) => m['version'] === 1); const v1Metadata = metadatas.find((m: any) => m['version'] === 1);
let v2Metadata = metadatas.find((m: any) => m['version'] === 2); let v3Metadata = metadatas.find((m: any) => m['version'] === 3);
if (!v2Metadata && v1Metadata) { if (!v3Metadata && v1Metadata) {
// patch up v1 to v2 by merging the metadata with metadata collected from the d.ts file // 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 // as the only difference between the versions is whether all exports are contained in
// the metadata and the `extends` clause. // the metadata and the `extends` clause.
v2Metadata = {'__symbolic': 'module', 'version': 2, 'metadata': {}}; v3Metadata = {'__symbolic': 'module', 'version': 3, 'metadata': {}};
if (v1Metadata.exports) { if (v1Metadata.exports) {
v2Metadata.exports = v1Metadata.exports; v3Metadata.exports = v1Metadata.exports;
} }
for (let prop in v1Metadata.metadata) { 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)); const exports = this.metadataCollector.getMetadata(this.getSourceFile(dtsFilePath));
if (exports) { if (exports) {
for (let prop in exports.metadata) { for (let prop in exports.metadata) {
if (!v2Metadata.metadata[prop]) { if (!v3Metadata.metadata[prop]) {
v2Metadata.metadata[prop] = exports.metadata[prop]; v3Metadata.metadata[prop] = exports.metadata[prop];
} }
} }
if (exports.exports) { if (exports.exports) {
v2Metadata.exports = exports.exports; v2Metadata.exports = exports.exports;
} }
} }
metadatas.push(v2Metadata); metadatas.push(v3Metadata);
} }
this.resolverCache.set(filePath, metadatas); this.resolverCache.set(filePath, metadatas);
return metadatas; return metadatas;

View File

@ -166,11 +166,11 @@ describe('CompilerHost', () => {
expect(hostNestedGenDir.getMetadataFor('node_modules/@angular/missing.d.ts')).toBeUndefined(); 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([ expect(hostNestedGenDir.getMetadataFor('metadata_versions/v1.d.ts')).toEqual([
{__symbolic: 'module', version: 1, metadata: {foo: {__symbolic: 'class'}}}, { {__symbolic: 'module', version: 1, metadata: {foo: {__symbolic: 'class'}}}, {
__symbolic: 'module', __symbolic: 'module',
version: 2, version: 3,
metadata: { metadata: {
foo: {__symbolic: 'class'}, foo: {__symbolic: 'class'},
Bar: {__symbolic: 'class', members: {ngOnInit: [{__symbolic: 'method'}]}}, Bar: {__symbolic: 'class', members: {ngOnInit: [{__symbolic: 'method'}]}},

View File

@ -10,7 +10,7 @@ import {Attribute, Component, ContentChild, ContentChildren, Directive, Host, Ho
import {ReflectorReader} from '../private_import_core'; import {ReflectorReader} from '../private_import_core';
import {StaticSymbol} from './static_symbol'; import {StaticSymbol} from './static_symbol';
const SUPPORTED_SCHEMA_VERSION = 2; const SUPPORTED_SCHEMA_VERSION = 3;
const ANGULAR_IMPORT_LOCATIONS = { const ANGULAR_IMPORT_LOCATIONS = {
coreDecorators: '@angular/core/src/metadata', coreDecorators: '@angular/core/src/metadata',
diDecorators: '@angular/core/src/di/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: {}}; {__symbolic: 'module', version: SUPPORTED_SCHEMA_VERSION, module: module, metadata: {}};
} }
if (moduleMetadata['version'] != SUPPORTED_SCHEMA_VERSION) { if (moduleMetadata['version'] != SUPPORTED_SCHEMA_VERSION) {
this.reportError( const errorMessage = moduleMetadata['version'] == 2 ?
new Error( `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}`), `Metadata version mismatch for module ${module}, found version ${moduleMetadata['version']}, expected ${SUPPORTED_SCHEMA_VERSION}`;
null); this.reportError(new Error(errorMessage), null);
} }
this.metadataCache.set(module, moduleMetadata); this.metadataCache.set(module, moduleMetadata);
} }

View File

@ -80,7 +80,13 @@ describe('StaticReflector', () => {
it('should throw an exception for unsupported metadata versions', () => { it('should throw an exception for unsupported metadata versions', () => {
expect(() => reflector.findDeclaration('src/version-error', 'e')) expect(() => reflector.findDeclaration('src/version-error', 'e'))
.toThrow(new Error( .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', () => { 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'); const metadata = reflector.getModuleMetadata('/tmp/src/custom-decorator-reference.ts');
expect(metadata).toEqual({ expect(metadata).toEqual({
__symbolic: 'module', __symbolic: 'module',
version: 2, version: 3,
metadata: { metadata: {
Foo: { Foo: {
__symbolic: 'class', __symbolic: 'class',
@ -775,7 +781,7 @@ export class MockStaticReflectorHost implements StaticReflectorHost {
const DEFAULT_TEST_DATA: {[key: string]: any} = { const DEFAULT_TEST_DATA: {[key: string]: any} = {
'/tmp/@angular/common/src/forms-deprecated/directives.d.ts': [{ '/tmp/@angular/common/src/forms-deprecated/directives.d.ts': [{
'__symbolic': 'module', '__symbolic': 'module',
'version': 2, 'version': 3,
'metadata': { 'metadata': {
'FORM_DIRECTIVES': [ 'FORM_DIRECTIVES': [
{ {
@ -788,7 +794,7 @@ const DEFAULT_TEST_DATA: {[key: string]: any} = {
}], }],
'/tmp/@angular/common/src/directives/ng_for.d.ts': { '/tmp/@angular/common/src/directives/ng_for.d.ts': {
'__symbolic': 'module', '__symbolic': 'module',
'version': 2, 'version': 3,
'metadata': { 'metadata': {
'NgFor': { 'NgFor': {
'__symbolic': 'class', '__symbolic': 'class',
@ -841,16 +847,16 @@ const DEFAULT_TEST_DATA: {[key: string]: any} = {
} }
}, },
'/tmp/@angular/core/src/linker/view_container_ref.d.ts': '/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': '/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': '/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': '/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': { '/tmp/src/app/hero-detail.component.d.ts': {
'__symbolic': 'module', '__symbolic': 'module',
'version': 2, 'version': 3,
'metadata': { 'metadata': {
'HeroDetailComponent': { 'HeroDetailComponent': {
'__symbolic': 'class', '__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-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': { '/tmp/src/error-reporting.d.ts': {
__symbolic: 'module', __symbolic: 'module',
version: 2, version: 3,
metadata: { metadata: {
SomeClass: { SomeClass: {
__symbolic: 'class', __symbolic: 'class',
@ -1035,7 +1042,7 @@ const DEFAULT_TEST_DATA: {[key: string]: any} = {
}, },
'/tmp/src/error-references.d.ts': { '/tmp/src/error-references.d.ts': {
__symbolic: 'module', __symbolic: 'module',
version: 2, version: 3,
metadata: { metadata: {
Link1: { Link1: {
__symbolic: 'reference', __symbolic: 'reference',
@ -1057,7 +1064,7 @@ const DEFAULT_TEST_DATA: {[key: string]: any} = {
}, },
'/tmp/src/function-declaration.d.ts': { '/tmp/src/function-declaration.d.ts': {
__symbolic: 'module', __symbolic: 'module',
version: 2, version: 3,
metadata: { metadata: {
one: { one: {
__symbolic: 'function', __symbolic: 'function',
@ -1086,7 +1093,7 @@ const DEFAULT_TEST_DATA: {[key: string]: any} = {
}, },
'/tmp/src/function-reference.ts': { '/tmp/src/function-reference.ts': {
__symbolic: 'module', __symbolic: 'module',
version: 2, version: 3,
metadata: { metadata: {
one: { one: {
__symbolic: 'call', __symbolic: 'call',
@ -1128,7 +1135,7 @@ const DEFAULT_TEST_DATA: {[key: string]: any} = {
}, },
'/tmp/src/function-recursive.d.ts': { '/tmp/src/function-recursive.d.ts': {
__symbolic: 'modules', __symbolic: 'modules',
version: 2, version: 3,
metadata: { metadata: {
recursive: { recursive: {
__symbolic: 'function', __symbolic: 'function',
@ -1188,7 +1195,7 @@ const DEFAULT_TEST_DATA: {[key: string]: any} = {
}, },
'/tmp/src/spread.ts': { '/tmp/src/spread.ts': {
__symbolic: 'module', __symbolic: 'module',
version: 2, version: 3,
metadata: { metadata: {
spread: [0, {__symbolic: 'spread', expression: [1, 2, 3, 4]}, 5] 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': { '/tmp/src/reexport/reexport.d.ts': {
__symbolic: 'module', __symbolic: 'module',
version: 2, version: 3,
metadata: {}, metadata: {},
exports: [ exports: [
{from: './src/origin1', export: ['One', 'Two', {name: 'Three', as: 'Four'}]}, {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': { '/tmp/src/reexport/src/origin1.d.ts': {
__symbolic: 'module', __symbolic: 'module',
version: 2, version: 3,
metadata: { metadata: {
One: {__symbolic: 'class'}, One: {__symbolic: 'class'},
Two: {__symbolic: 'class'}, Two: {__symbolic: 'class'},
@ -1356,26 +1363,26 @@ const DEFAULT_TEST_DATA: {[key: string]: any} = {
}, },
'/tmp/src/reexport/src/origin5.d.ts': { '/tmp/src/reexport/src/origin5.d.ts': {
__symbolic: 'module', __symbolic: 'module',
version: 2, version: 3,
metadata: { metadata: {
Five: {__symbolic: 'class'}, Five: {__symbolic: 'class'},
}, },
}, },
'/tmp/src/reexport/src/origin30.d.ts': { '/tmp/src/reexport/src/origin30.d.ts': {
__symbolic: 'module', __symbolic: 'module',
version: 2, version: 3,
metadata: { metadata: {
Thirty: {__symbolic: 'class'}, Thirty: {__symbolic: 'class'},
}, },
}, },
'/tmp/src/reexport/src/originNone.d.ts': { '/tmp/src/reexport/src/originNone.d.ts': {
__symbolic: 'module', __symbolic: 'module',
version: 2, version: 3,
metadata: {}, metadata: {},
}, },
'/tmp/src/reexport/src/reexport2.d.ts': { '/tmp/src/reexport/src/reexport2.d.ts': {
__symbolic: 'module', __symbolic: 'module',
version: 2, version: 3,
metadata: {}, metadata: {},
exports: [{from: './originNone'}, {from: './origin30'}] exports: [{from: './originNone'}, {from: './origin30'}]
} }

View File

@ -17,6 +17,11 @@ import {Symbols} from './symbols';
* A set of collector options to use when collecting metadata. * A set of collector options to use when collecting metadata.
*/ */
export class CollectorOptions { 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 * Collect a hidden field "$quoted$" in objects literals that record when the key was quoted in
* the source. * the source.
@ -430,7 +435,10 @@ export class MetadataCollector {
else if (strict) { else if (strict) {
validateMetadata(sourceFile, nodeMap, metadata); 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; if (exports) result.exports = exports;
return result; return result;
} }

View File

@ -110,7 +110,8 @@ export class TsickleCompilerHost extends DelegatingHost {
const IGNORED_FILES = /\.ngfactory\.js$|\.ngstyle\.js$/; const IGNORED_FILES = /\.ngfactory\.js$|\.ngstyle\.js$/;
export class MetadataWriterHost extends DelegatingHost { 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); } constructor(delegate: ts.CompilerHost, private ngOptions: NgOptions) { super(delegate); }
private writeMetadata(emitFilePath: string, sourceFile: ts.SourceFile) { 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 path = emitFilePath.replace(/*DTS*/ /\.js$/, '.metadata.json');
const metadata = const metadata =
this.metadataCollector.getMetadata(sourceFile, !!this.ngOptions.strictMetadataEmit); this.metadataCollector.getMetadata(sourceFile, !!this.ngOptions.strictMetadataEmit);
const metadatas: ModuleMetadata[] = [metadata]; const metadata1 = this.metadataCollector1.getMetadata(sourceFile, false);
if (metadata && metadata.metadata) { const metadatas: ModuleMetadata[] = [metadata, metadata1].filter(e => !!e);
if (metadata.version === 2) { if (metadatas.length) {
// 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 metadataText = JSON.stringify(metadatas); const metadataText = JSON.stringify(metadatas);
writeFileSync(path, metadataText, {encoding: 'utf-8'}); writeFileSync(path, metadataText, {encoding: 'utf-8'});
} }

View File

@ -15,7 +15,7 @@
// semantics of the file in an array. For example, when generating a version 2 file, if version 1 // 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. // 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; export type MetadataEntry = ClassMetadata | FunctionMetadata | MetadataValue;

View File

@ -67,7 +67,7 @@ describe('Collector', () => {
const metadata = collector.getMetadata(sourceFile); const metadata = collector.getMetadata(sourceFile);
expect(metadata).toEqual({ expect(metadata).toEqual({
__symbolic: 'module', __symbolic: 'module',
version: 2, version: 3,
metadata: { metadata: {
HeroDetailComponent: { HeroDetailComponent: {
__symbolic: 'class', __symbolic: 'class',
@ -108,7 +108,7 @@ describe('Collector', () => {
const metadata = collector.getMetadata(sourceFile); const metadata = collector.getMetadata(sourceFile);
expect(metadata).toEqual({ expect(metadata).toEqual({
__symbolic: 'module', __symbolic: 'module',
version: 2, version: 3,
metadata: { metadata: {
AppComponent: { AppComponent: {
__symbolic: 'class', __symbolic: 'class',
@ -162,7 +162,7 @@ describe('Collector', () => {
const metadata = collector.getMetadata(sourceFile); const metadata = collector.getMetadata(sourceFile);
expect(metadata).toEqual({ expect(metadata).toEqual({
__symbolic: 'module', __symbolic: 'module',
version: 2, version: 3,
metadata: { metadata: {
HEROES: [ HEROES: [
{'id': 11, 'name': 'Mr. Nice', '$quoted$': ['id', 'name']}, {'id': 11, 'name': 'Mr. Nice', '$quoted$': ['id', 'name']},
@ -241,7 +241,7 @@ describe('Collector', () => {
const metadata = collector.getMetadata(unsupported1); const metadata = collector.getMetadata(unsupported1);
expect(metadata).toEqual({ expect(metadata).toEqual({
__symbolic: 'module', __symbolic: 'module',
version: 2, version: 3,
metadata: { metadata: {
a: {__symbolic: 'error', message: 'Destructuring not supported', line: 1, character: 16}, a: {__symbolic: 'error', message: 'Destructuring not supported', line: 1, character: 16},
b: {__symbolic: 'error', message: 'Destructuring not supported', line: 1, character: 19}, b: {__symbolic: 'error', message: 'Destructuring not supported', line: 1, character: 19},
@ -283,7 +283,7 @@ describe('Collector', () => {
const metadata = collector.getMetadata(sourceFile); const metadata = collector.getMetadata(sourceFile);
expect(metadata).toEqual({ expect(metadata).toEqual({
__symbolic: 'module', __symbolic: 'module',
version: 2, version: 3,
metadata: { metadata: {
SimpleClass: {__symbolic: 'class'}, SimpleClass: {__symbolic: 'class'},
AbstractClass: {__symbolic: 'class'}, AbstractClass: {__symbolic: 'class'},
@ -297,7 +297,7 @@ describe('Collector', () => {
const metadata = collector.getMetadata(exportedFunctions); const metadata = collector.getMetadata(exportedFunctions);
expect(metadata).toEqual({ expect(metadata).toEqual({
__symbolic: 'module', __symbolic: 'module',
version: 2, version: 3,
metadata: { metadata: {
one: { one: {
__symbolic: 'function', __symbolic: 'function',