Skip to content

Commit

Permalink
fix(metadata): Do not attach module names to metadata.
Browse files Browse the repository at this point in the history 
The filename contains the module name as resolved by users, so the top-level module name is uneeded.
Module names on references are replaced by capturing the import syntax from the module.
This allows readers of the metadata to do the module resolution themselves.

Fixes angular#8225
  • Loading branch information
@alexeagle
alexeagle committed Apr 26, 2016
1 parent 5a897cf commit 9992fee
Show file tree
Hide file tree
Showing 8 changed files with 212 additions and 218 deletions.
70 changes: 18 additions & 52 deletions modules/angular2/src/compiler/static_reflector.ts
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,8 @@
import {ListWrapper, StringMapWrapper} from 'angular2/src/facade/collection';
import {StringMapWrapper} from 'angular2/src/facade/collection';
import {
isArray,
isBlank,
isNumber,
isPresent,
isPrimitive,
isString,
Type
} from 'angular2/src/facade/lang';
import {
AttributeMetadata,
Expand Down Expand Up @@ -35,13 +31,19 @@ import {ReflectorReader} from 'angular2/src/core/reflection/reflector_reader';
*/
export interface StaticReflectorHost {
/**
* Return a ModuleMetadata for the give module.
* Return a ModuleMetadata for the given module.
*
* @param moduleId is a string identifier for a module in the form that would expected in a
* module import of an import statement.
* @param moduleId is a string identifier for a module as an absolute path.
* @returns the metadata for the given module.
*/
getMetadataFor(moduleId: string): {[key: string]: any};

/**
* Resolve a module from an import statement form to an absolute path.
* @param moduleName the location imported from
* @param containingFile for relative imports, the path of the file containing the import
*/
resolveModule(moduleName: string, containingFile?: string): string;
}

/**
Expand All @@ -68,10 +70,10 @@ export class StaticReflector implements ReflectorReader {
importUri(typeOrFunc: any): string { return (<StaticType>typeOrFunc).moduleId; }

/**
* getStatictype produces a Type whose metadata is known but whose implementation is not loaded.
* getStaticType produces a Type whose metadata is known but whose implementation is not loaded.
* All types passed to the StaticResolver should be pseudo-types returned by this method.
*
* @param moduleId the module identifier as would be passed to an import statement.
* @param moduleId the module identifier as an absolute path.
* @param name the name of the type.
*/
public getStaticType(moduleId: string, name: string): StaticType {
Expand Down Expand Up @@ -137,7 +139,7 @@ export class StaticReflector implements ReflectorReader {

private conversionMap = new Map<StaticType, (moduleContext: string, expression: any) => any>();
private initializeConversionMap(): any {
let core_metadata = 'angular2/src/core/metadata';
let core_metadata = this.host.resolveModule('angular2/src/core/metadata');
let conversionMap = this.conversionMap;
conversionMap.set(this.getStaticType(core_metadata, 'Directive'),
(moduleContext, expression) => {
Expand Down Expand Up @@ -272,7 +274,7 @@ export class StaticReflector implements ReflectorReader {
if (isMetadataSymbolicCallExpression(expression)) {
let target = expression['expression'];
if (isMetadataSymbolicReferenceExpression(target)) {
let moduleId = this.normalizeModuleName(moduleContext, target['module']);
let moduleId = this.host.resolveModule(target['module'], moduleContext);
return this.getStaticType(moduleId, target['name']);
}
}
Expand Down Expand Up @@ -417,7 +419,7 @@ export class StaticReflector implements ReflectorReader {
return null;
case "reference":
let referenceModuleName =
_this.normalizeModuleName(moduleContext, expression['module']);
_this.host.resolveModule(expression['module'], moduleContext);
let referenceModule = _this.getModuleMetadata(referenceModuleName);
let referenceValue = referenceModule['metadata'][expression['name']];
if (isClassMetadata(referenceValue)) {
Expand All @@ -440,6 +442,9 @@ export class StaticReflector implements ReflectorReader {
return simplify(value);
}

/**
* @param module an absolute path to a module file.
*/
public getModuleMetadata(module: string): {[key: string]: any} {
let moduleMetadata = this.metadataCache.get(module);
if (!isPresent(moduleMetadata)) {
Expand All @@ -460,13 +465,6 @@ export class StaticReflector implements ReflectorReader {
}
return result;
}

private normalizeModuleName(from: string, to: string): string {
if (to.startsWith('.')) {
return pathTo(from, to);
}
return to;
}
}

function isMetadataSymbolicCallExpression(expression: any): boolean {
Expand All @@ -481,35 +479,3 @@ function isMetadataSymbolicReferenceExpression(expression: any): boolean {
function isClassMetadata(expression: any): boolean {
return !isPrimitive(expression) && !isArray(expression) && expression['__symbolic'] == 'class';
}

function splitPath(path: string): string[] {
return path.split(/\/|\\/g);
}

function resolvePath(pathParts: string[]): string {
let result = [];
ListWrapper.forEachWithIndex(pathParts, (part, index) => {
switch (part) {
case '':
case '.':
if (index > 0) return;
break;
case '..':
if (index > 0 && result.length != 0) result.pop();
return;
}
result.push(part);
});
return result.join('/');
}

function pathTo(from: string, to: string): string {
let result = to;
if (to.startsWith('.')) {
let fromParts = splitPath(from);
fromParts.pop(); // remove the file name.
let toParts = splitPath(to);
result = resolvePath(fromParts.concat(toParts));
}
return result;
}
118 changes: 70 additions & 48 deletions modules/angular2/test/compiler/static_reflector_spec.ts
View file
Original file line number Diff line number Diff line change
@@ -1,27 +1,20 @@
import {
ddescribe,
describe,
xdescribe,
it,
iit,
xit,
expect,
beforeEach,
afterEach,
AsyncTestCompleter,
inject,
beforeEachProviders
} from 'angular2/testing_internal';
import {ListWrapper} from 'angular2/src/facade/collection';

import {StaticReflector, StaticReflectorHost} from 'angular2/src/compiler/static_reflector';

export function main() {
describe('StaticRefelector', () => {
describe('StaticReflector', () => {
it('should get annotations for NgFor', () => {
let host = new MockReflectorHost();
let reflector = new StaticReflector(host);

let NgFor = reflector.getStaticType('angular2/src/common/directives/ng_for', 'NgFor');
let NgFor = reflector.getStaticType(
host.resolveModule('angular2/src/common/directives/ng_for'), 'NgFor');
let annotations = reflector.annotations(NgFor);
expect(annotations.length).toEqual(1);
let annotation = annotations[0];
Expand All @@ -33,15 +26,18 @@ export function main() {
let host = new MockReflectorHost();
let reflector = new StaticReflector(host);

let NgFor = reflector.getStaticType('angular2/src/common/directives/ng_for', 'NgFor');
let ViewContainerRef = reflector.getStaticType('angular2/src/core/linker/view_container_ref',
'ViewContainerRef');
let TemplateRef =
reflector.getStaticType('angular2/src/core/linker/template_ref', 'TemplateRef');
let NgFor = reflector.getStaticType(
host.resolveModule('angular2/src/common/directives/ng_for'), 'NgFor');
let ViewContainerRef = reflector.getStaticType(
host.resolveModule('angular2/src/core/linker/view_container_ref'), 'ViewContainerRef');
let TemplateRef = reflector.getStaticType(
host.resolveModule('angular2/src/core/linker/template_ref'), 'TemplateRef');
let IterableDiffers = reflector.getStaticType(
'angular2/src/core/change_detection/differs/iterable_differs', 'IterableDiffers');
host.resolveModule('angular2/src/core/change_detection/differs/iterable_differs'),
'IterableDiffers');
let ChangeDetectorRef = reflector.getStaticType(
'angular2/src/core/change_detection/change_detector_ref', 'ChangeDetectorRef');
host.resolveModule('angular2/src/core/change_detection/change_detector_ref'),
'ChangeDetectorRef');

let parameters = reflector.parameters(NgFor);
expect(parameters)
Expand All @@ -53,7 +49,7 @@ export function main() {
let reflector = new StaticReflector(host);

let HeroDetailComponent =
reflector.getStaticType('./app/hero-detail.component', 'HeroDetailComponent');
reflector.getStaticType('/src/app/hero-detail.component.ts', 'HeroDetailComponent');
let annotations = reflector.annotations(HeroDetailComponent);
expect(annotations.length).toEqual(1);
let annotation = annotations[0];
Expand All @@ -64,7 +60,7 @@ export function main() {
let host = new MockReflectorHost();
let reflector = new StaticReflector(host);

let UnknownClass = reflector.getStaticType('./app/app.component', 'UnknownClass');
let UnknownClass = reflector.getStaticType('/src/app/app.component.ts', 'UnknownClass');
let annotations = reflector.annotations(UnknownClass);
expect(annotations).toEqual([]);
});
Expand All @@ -74,7 +70,7 @@ export function main() {
let reflector = new StaticReflector(host);

let HeroDetailComponent =
reflector.getStaticType('./app/hero-detail.component', 'HeroDetailComponent');
reflector.getStaticType('/src/app/hero-detail.component.ts', 'HeroDetailComponent');
let props = reflector.propMetadata(HeroDetailComponent);
expect(props['hero']).toBeTruthy();
});
Expand All @@ -83,7 +79,7 @@ export function main() {
let host = new MockReflectorHost();
let reflector = new StaticReflector(host);

let UnknownClass = reflector.getStaticType('./app/app.component', 'UnknownClass');
let UnknownClass = reflector.getStaticType('/src/app/app.component.ts', 'UnknownClass');
let properties = reflector.propMetadata(UnknownClass);
expect(properties).toEqual({});
});
Expand All @@ -92,7 +88,7 @@ export function main() {
let host = new MockReflectorHost();
let reflector = new StaticReflector(host);

let UnknownClass = reflector.getStaticType('./app/app.component', 'UnknownClass');
let UnknownClass = reflector.getStaticType('/src/app/app.component.ts', 'UnknownClass');
let parameters = reflector.parameters(UnknownClass);
expect(parameters).toEqual([]);
});
Expand Down Expand Up @@ -301,7 +297,7 @@ export function main() {
expect(reflector.simplify('', ({ __symbolic: 'pre', operator: '!', operand: false}))).toBe(!false);
});

it('should simpify an array index', () => {
it('should simplify an array index', () => {
let host = new MockReflectorHost();
let reflector = new StaticReflector(host);

Expand All @@ -320,20 +316,56 @@ export function main() {
let host = new MockReflectorHost();
let reflector = new StaticReflector(host);

expect(
reflector.simplify('./cases', ({__symbolic: "reference", module: "./extern", name: "s"})))
expect(reflector.simplify('/src/cases',
({__symbolic: "reference", module: "./extern", name: "s"})))
.toEqual("s");
});
});
}

class MockReflectorHost implements StaticReflectorHost {
resolveModule(moduleName: string, containingFile?: string): string {
function splitPath(path: string): string[] { return path.split(/\/|\\/g); }

function resolvePath(pathParts: string[]): string {
let result = [];
ListWrapper.forEachWithIndex(pathParts, (part, index) => {
switch (part) {
case '':
case '.':
if (index > 0) return;
break;
case '..':
if (index > 0 && result.length != 0) result.pop();
return;
}
result.push(part);
});
return result.join('/');
}

function pathTo(from: string, to: string): string {
let result = to;
if (to.startsWith('.')) {
let fromParts = splitPath(from);
fromParts.pop(); // remove the file name.
let toParts = splitPath(to);
result = resolvePath(fromParts.concat(toParts));
}
return result;
}

if (moduleName.indexOf('.') === 0) {
return pathTo(containingFile, moduleName) + '.d.ts';
}
return '/tmp/' + moduleName + '.d.ts';
}

getMetadataFor(moduleId: string): any {
return {
'angular2/src/common/directives/ng_for':
'/tmp/angular2/src/common/directives/ng_for.d.ts':
{
"__symbolic": "module",
"module": "./ng_for",
"metadata":
{
"NgFor":
Expand Down Expand Up @@ -393,27 +425,17 @@ class MockReflectorHost implements StaticReflectorHost {
}
}
},
'angular2/src/core/linker/view_container_ref':
{
"module": "./view_container_ref",
"metadata": {"ViewContainerRef": {"__symbolic": "class"}}
},
'angular2/src/core/linker/template_ref':
'/tmp/angular2/src/core/linker/view_container_ref.d.ts':
{"metadata": {"ViewContainerRef": {"__symbolic": "class"}}},
'/tmp/angular2/src/core/linker/template_ref.d.ts':
{"module": "./template_ref", "metadata": {"TemplateRef": {"__symbolic": "class"}}},
'angular2/src/core/change_detection/differs/iterable_differs':
{
"module": "./iterable_differs",
"metadata": {"IterableDiffers": {"__symbolic": "class"}}
},
'angular2/src/core/change_detection/change_detector_ref':
{
"module": "./change_detector_ref",
"metadata": {"ChangeDetectorRef": {"__symbolic": "class"}}
},
'./app/hero-detail.component':
'/tmp/angular2/src/core/change_detection/differs/iterable_differs.d.ts':
{"metadata": {"IterableDiffers": {"__symbolic": "class"}}},
'/tmp/angular2/src/core/change_detection/change_detector_ref.d.ts':
{"metadata": {"ChangeDetectorRef": {"__symbolic": "class"}}},
'/src/app/hero-detail.component.ts':
{
"__symbolic": "module",
"module": "./hero-detail.component",
"metadata":
{
"HeroDetailComponent":
Expand Down Expand Up @@ -459,8 +481,8 @@ class MockReflectorHost implements StaticReflectorHost {
}
}
},
'./extern': {
"__symbolic": "module", module: './extern', metadata: { s: "s" }
'/src/extern.d.ts': {
"__symbolic": "module", metadata: { s: "s" }
}
}
[moduleId];
Expand Down
11 changes: 1 addition & 10 deletions tools/broccoli/broccoli-typescript.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,16 +88,7 @@ class DiffingTSCompiler implements DiffingBroccoliPlugin {
this.tsServiceHost = new CustomLanguageServiceHost(this.tsOpts, this.rootFilePaths,
this.fileRegistry, this.inputPath);
this.tsService = ts.createLanguageService(this.tsServiceHost, ts.createDocumentRegistry());
this.metadataCollector = new MetadataCollector({
// Since our code isn't under a node_modules directory, we need to reverse the module
// resolution to get metadata rooted at 'angular2'.
// see https://github.com/angular/angular/issues/8144
reverseModuleResolution(fileName: string) {
if (/\.tmp\/angular2/.test(fileName)) {
return fileName.substr(fileName.lastIndexOf('.tmp/angular2/') + 5).replace(/\.ts$/, '');
}
}
});
this.metadataCollector = new MetadataCollector();
}


Expand Down
Loading

0 comments on commit 9992fee

Please sign in to comment.