Skip to content

Commit

Permalink
fix(compiler): update to metadata version 3 (#13464)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@chuckjaz @vicb
chuckjaz authored and vicb committed Dec 15, 2016
1 parent a72a002 commit b9b557c
Show file tree
Hide file tree
Showing 8 changed files with 66 additions and 59 deletions.
18 changes: 9 additions & 9 deletions modules/@angular/compiler-cli/src/compiler_host.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions modules/@angular/compiler-cli/test/aot_host_spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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'}]}},
Expand Down
10 changes: 5 additions & 5 deletions modules/@angular/compiler/src/aot/static_reflector.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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);
}
Expand Down
51 changes: 29 additions & 22 deletions modules/@angular/compiler/test/aot/static_reflector_spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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': [
{
Expand All @@ -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',
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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',
Expand All @@ -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',
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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]
}
Expand Down Expand Up @@ -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'}]},
Expand All @@ -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'},
Expand All @@ -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'}]
}
Expand Down
10 changes: 9 additions & 1 deletion tools/@angular/tsc-wrapped/src/collector.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
}
Expand Down
18 changes: 5 additions & 13 deletions tools/@angular/tsc-wrapped/src/compiler_host.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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'});
}
Expand Down
2 changes: 1 addition & 1 deletion tools/@angular/tsc-wrapped/src/schema.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
12 changes: 6 additions & 6 deletions tools/@angular/tsc-wrapped/test/collector.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ describe('Collector', () => {
const metadata = collector.getMetadata(sourceFile);
expect(metadata).toEqual({
__symbolic: 'module',
version: 2,
version: 3,
metadata: {
HeroDetailComponent: {
__symbolic: 'class',
Expand Down Expand Up @@ -108,7 +108,7 @@ describe('Collector', () => {
const metadata = collector.getMetadata(sourceFile);
expect(metadata).toEqual({
__symbolic: 'module',
version: 2,
version: 3,
metadata: {
AppComponent: {
__symbolic: 'class',
Expand Down Expand Up @@ -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']},
Expand Down Expand Up @@ -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},
Expand Down Expand Up @@ -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'},
Expand All @@ -297,7 +297,7 @@ describe('Collector', () => {
const metadata = collector.getMetadata(exportedFunctions);
expect(metadata).toEqual({
__symbolic: 'module',
version: 2,
version: 3,
metadata: {
one: {
__symbolic: 'function',
Expand Down

0 comments on commit b9b557c

Please sign in to comment.