Skip to content
This repository was archived by the owner on Feb 22, 2018. It is now read-only.

Commit 8d36167

Browse files
Ted Sanderrkirov
Ted Sander
authored and
rkirov
committed
fix(transformers): Fix relative URL lookup for transformers, and web
Extracts logic used in the TypeRelativeUriGenerator to be used in other transformers. This allows other transformers such as the ExpressionGenerator and later the TemplateCacheGenerator to find relative resources. Fixes asset resolution according to Barback rules which are defined here: http://goo.gl/YDMRc2 specifically library assets are defined with packages being in the URI and all other resources are considered web resources. Closes #1649, #1651
1 parent f55b6bc commit 8d36167

File tree

4 files changed

+107
-29
lines changed

4 files changed

+107
-29
lines changed

‎lib/tools/transformer/referenced_uris.dart

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import 'dart:async';
44

55
import 'package:analyzer/src/generated/ast.dart';
66
import 'package:analyzer/src/generated/element.dart';
7+
import 'package:angular/tools/transformer/transformer_resource_url_resolver.dart';
78
import 'package:angular/tools/transformer/options.dart';
89
import 'package:barback/barback.dart';
910
import 'package:code_transformers/resolver.dart';
@@ -15,8 +16,10 @@ import 'package:path/path.dart' as path;
1516
Future<Map<String, String>> gatherReferencedUris(Transform transform,
1617
Resolver resolver, TransformOptions options,
1718
{bool skipNonCached: false, bool templatesOnly: false}) {
19+
var urlResolver =
20+
new TransformerResourceUrlResolver(resolver, transform.primaryInput.id);
1821
return new _Processor(transform, resolver, options, skipNonCached,
19-
templatesOnly).process();
22+
templatesOnly, urlResolver).process();
2023
}
2124

2225
class _Processor {
@@ -26,6 +29,7 @@ class _Processor {
2629
final Map<RegExp, String> templateUriRewrites = <RegExp, String>{};
2730
final bool skipNonCached;
2831
final bool templatesOnly;
32+
final TransformerResourceUrlResolver urlResolver;
2933

3034
ConstructorElement cacheAnnotation;
3135
ConstructorElement componentAnnotation;
@@ -35,7 +39,7 @@ class _Processor {
3539
static const String componentAnnotationName = 'angular.core.annotation_src.Component';
3640

3741
_Processor(this.transform, this.resolver, this.options, this.skipNonCached,
38-
this.templatesOnly) {
42+
this.templatesOnly, this.urlResolver) {
3943
for (var key in options.templateUriRewrites.keys) {
4044
templateUriRewrites[new RegExp(key)] = options.templateUriRewrites[key];
4145
}
@@ -199,6 +203,7 @@ class _Processor {
199203

200204
_CacheEntry uriToEntry(String uri, Element reference) {
201205
uri = rewriteUri(uri);
206+
uri = urlResolver.combineWithElement(reference, uri);
202207
if (Uri.parse(uri).scheme != '') {
203208
warn('Cannot cache non-local URIs. $uri', reference);
204209
return null;
@@ -212,7 +217,10 @@ class _Processor {
212217
warn('Cannot cache non-package absolute URIs. $uri', reference);
213218
return null;
214219
}
215-
var assetId = new AssetId(transform.primaryInput.id.package, uri);
220+
// Everything else is a resource in the web directory according to pub;
221+
// as all packages URIs were handled above. As specified in this
222+
// [Barback Doc](http://goo.gl/YDMRc2)
223+
var assetId = new AssetId(transform.primaryInput.id.package, 'web/$uri');
216224
return new _CacheEntry(uri, reference, assetId);
217225
}
218226

‎lib/tools/transformer/transformer_resource_url_resolver.dart

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
library angular.tools.transformer.angular_file_resolver;
2+
3+
import 'package:analyzer/src/generated/element.dart';
4+
import 'package:code_transformers/resolver.dart';
5+
import 'package:barback/barback.dart';
6+
7+
/// Resolver used to find absolute resources when in Transformers by combining
8+
/// AST element locations and relative URIs.
9+
class TransformerResourceUrlResolver {
10+
final Resolver _resolver;
11+
final AssetId _primaryAsset;
12+
13+
TransformerResourceUrlResolver(this._resolver, this._primaryAsset);
14+
15+
Uri findUriOfElement(Element type) {
16+
var uri = _resolver.getImportUri(type.library, from: _primaryAsset);
17+
var acceptable = (
18+
(uri.isAbsolute && uri.scheme == 'package') ||
19+
(uri.toString() == uri.path));
20+
if (!acceptable) {
21+
var errMsg = 'ERROR: Type "$type" has unsupported URI $uri';
22+
throw errMsg;
23+
}
24+
if (uri.scheme != "package") {
25+
// this is guaranteed to be a relative URL (e.g. type defined in a path
26+
// imported file)
27+
var path = _primaryAsset.path;
28+
if (!path.startsWith('web/')) {
29+
var errMsg = 'ERROR: Type "$type" is imported as a path not under web.';
30+
throw errMsg;
31+
}
32+
uri = Uri.parse(path.substring('web/'.length)).resolve(uri.path);
33+
}
34+
return uri;
35+
}
36+
37+
/// Given a AST [type] and [uri] if [uri] is relative combines it with the uri
38+
/// of the element to make an absolute location relative to types uri.
39+
/// Note: This logic should match [ResourceUrlResolver], but is separate as
40+
/// transformers and Mirrors have different APIs to identify elements, and
41+
/// calculate their URIs.
42+
String combineWithElement(Element type, String uri) {
43+
if (uri != null) {
44+
if (uri.startsWith("/")) return uri;
45+
if (uri.startsWith("packages/")) return "/" + uri;
46+
}
47+
48+
var typeUri = findUriOfElement(type);
49+
50+
if (uri == null) {
51+
uri = typeUri.path;
52+
}
53+
// If it's not absolute, then resolve it first
54+
Uri resolved = typeUri.resolve(uri);
55+
56+
if (resolved.scheme == 'package') {
57+
return '/packages/${resolved.path}';
58+
} else {
59+
return resolved.toString();
60+
}
61+
}
62+
}

‎lib/tools/transformer/type_relative_uri_generator.dart

Lines changed: 3 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import 'dart:async';
44
import 'package:analyzer/src/generated/ast.dart';
55
import 'package:analyzer/src/generated/element.dart';
66
import 'package:angular/tools/transformer/options.dart';
7+
import 'package:angular/tools/transformer/transformer_resource_url_resolver.dart';
78
import 'package:barback/barback.dart';
89
import 'package:code_transformers/resolver.dart';
910
import 'package:path/path.dart' as path;
@@ -36,6 +37,7 @@ class TypeRelativeUriGenerator extends Transformer with ResolverTransformer {
3637
transform.logger.warning('Unable to resolve $componentAnnotationName.');
3738
}
3839

40+
var urlResolver = new TransformerResourceUrlResolver(resolver, id);
3941
var annotatedTypes = resolver.libraries
4042
.expand((lib) => lib.units)
4143
.expand((unit) => unit.types)
@@ -69,28 +71,7 @@ class TypeRelativeUriGenerator extends Transformer with ResolverTransformer {
6971
for (var type in annotatedTypes) {
7072
outputBuffer.write(' ${importPrefixes[type.library]}${type.name}: ');
7173

72-
var uri = resolver.getImportUri(type.library,
73-
from: transform.primaryInput.id);
74-
75-
var acceptable = (
76-
(uri.isAbsolute && uri.scheme == "package") ||
77-
(uri.toString() == uri.path));
78-
if (!acceptable) {
79-
var errMsg = 'ERROR: $runtimeType: Type "$type" has unsupported URI $uri';
80-
transform.logger.error(errMsg);
81-
throw errMsg;
82-
}
83-
if (uri.scheme != "package") {
84-
// this is guaranteed to be a relative URL (e.g. type defined in a path
85-
// imported file)
86-
var path = transform.primaryInput.id.path;
87-
if (!path.startsWith("web/")) {
88-
var errMsg = 'ERROR: $runtimeType: Type "$type" is imported as a path not under web.';
89-
transform.logger.error(errMsg);
90-
throw errMsg;
91-
}
92-
uri = Uri.parse(path.substring("web/".length)).resolve(uri.path);
93-
}
74+
var uri = urlResolver.findUriOfElement(type);
9475
outputBuffer.write("Uri.parse(r'''$uri'''),\n");
9576
}
9677
_writeFooter(outputBuffer);

‎test/tools/transformer/expression_generator_spec.dart

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ main() {
6868
import 'package:angular/angular.dart';
6969
7070
@Component(
71-
templateUrl: 'lib/foo.html',
71+
templateUrl: 'foo.html',
7272
selector: 'my-component')
7373
class FooComponent {}
7474
@@ -79,7 +79,7 @@ main() {
7979
8080
main() {}
8181
''',
82-
'a|lib/foo.html': '''
82+
'a|web/foo.html': '''
8383
<div>{{template.contents}}</div>''',
8484
'b|lib/bar.html': '''
8585
<div>{{bar}}</div>''',
@@ -119,14 +119,41 @@ main() {
119119
symbols: []);
120120
});
121121

122+
it('should respect relative URLs', () {
123+
return generates(phases,
124+
inputs: {
125+
'a|web/main.dart': '''
126+
import 'package:b/bar.dart';
127+
128+
main() {}
129+
''',
130+
'b|lib/bar.dart': '''
131+
import 'package:angular/angular.dart';
132+
133+
@Component(
134+
templateUrl: 'bar.html',
135+
selector: 'my-component')
136+
class BarComponent {}
137+
''',
138+
'b|lib/bar.html': '''
139+
<div>{{bar}}</div>''',
140+
'a|web/index.html': '''
141+
<script src='main.dart' type='application/dart'></script>''',
142+
'angular|lib/angular.dart': libAngular,
143+
},
144+
getters: ['bar'],
145+
setters: ['bar'],
146+
symbols: []);
147+
});
148+
122149
it('should generate expressions for variables found in superclass', () {
123150
return generates(phases,
124151
inputs: {
125152
'a|web/main.dart': '''
126153
import 'package:angular/angular.dart';
127154
128155
@Component(
129-
templateUrl: 'lib/foo.html',
156+
templateUrl: 'foo.html',
130157
selector: 'my-component')
131158
class FooComponent extends BarComponent {
132159
@NgAttr('foo')
@@ -140,7 +167,7 @@ main() {
140167
141168
main() {}
142169
''',
143-
'a|lib/foo.html': '''
170+
'a|web/foo.html': '''
144171
<div>{{template.foo}}</div>
145172
<div>{{template.bar}}</div>''',
146173
'a|web/index.html': '''

0 commit comments

Comments
 (0)