pkg/polymer/lib/src/build/import_inliner.dart - Issue 180933002: combine script extractor and import inliner

Unified Diff: pkg/polymer/lib/src/build/import_inliner.dart

Issue 180933002: combine script extractor and import inliner (Closed) Base URL: https://dart.googlecode.com/svn/branches/bleeding_edge/dart

Patch Set: refactor Created 6 years, 10 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Download patch

« no previous file with comments | « pkg/polymer/lib/src/build/common.dart ('k') | pkg/polymer/lib/src/build/script_compactor.dart » ('j') | pkg/polymer/test/build/all_phases_test.dart » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

Index: pkg/polymer/lib/src/build/import_inliner.dart

diff --git a/pkg/polymer/lib/src/build/import_inliner.dart b/pkg/polymer/lib/src/build/import_inliner.dart

index 40d5ca58d197a51ebea3106ccf5fb457baad3470..b4e40613e8424f2bb45a199be87101afe7330c1b 100644

--- a/pkg/polymer/lib/src/build/import_inliner.dart

+++ b/pkg/polymer/lib/src/build/import_inliner.dart

@@ -8,14 +8,15 @@ library polymer.src.build.import_inliner;

import 'dart:async';

import 'dart:convert';

+import 'package:analyzer/src/generated/ast.dart';

import 'package:barback/barback.dart';

import 'package:path/path.dart' as path;

import 'package:html5lib/dom.dart' show

Document, DocumentFragment, Element, Node;

import 'package:html5lib/dom_parsing.dart' show TreeVisitor;

+import 'package:source_maps/refactor.dart' show TextEditTransaction;

import 'package:source_maps/span.dart';

-import 'code_extractor.dart'; // import just for documentation.

import 'common.dart';

class _HtmlInliner extends PolymerTransformer {

@@ -26,9 +27,6 @@ class _HtmlInliner extends PolymerTransformer {

final seen = new Set<AssetId>();

final scriptIds = <AssetId>[];

- static const TYPE_DART = 'application/dart';

- static const TYPE_JS = 'text/javascript';

_HtmlInliner(this.options, Transform transform)

: transform = transform,

logger = transform.logger,

@@ -38,21 +36,26 @@ class _HtmlInliner extends PolymerTransformer {

seen.add(docId);

Document document;

+ bool changed;

- return readPrimaryAsHtml(transform).then((document) =>

- _visitImports(document, docId).then((importsFound) {

+ return readPrimaryAsHtml(transform).then((doc) {

+ document = doc;

+ // Add the main script's ID, or null if none is present.

+ // This will be used by ScriptCompactor.

+ changed = _extractScripts(document.querySelectorAll('script'));

+ return _visitImports(document, docId);

+ }).then((importsFound) {

+ changed = changed || importsFound;

var output = transform.primaryInput;

- if (importsFound) {

- output = new Asset.fromString(docId, document.outerHtml);

- }

+ if (changed) output = new Asset.fromString(docId, document.outerHtml);

transform.addOutput(output);

// We produce a secondary asset with extra information for later phases.

transform.addOutput(new Asset.fromString(

docId.addExtension('.scriptUrls'),

JSON.encode(scriptIds, toEncodable: (id) => id.serialize())));

- }));

+ });

}

/// Visits imports in [document] and add the imported documents to documents.

@@ -107,7 +110,7 @@ class _HtmlInliner extends PolymerTransformer {

var insertionPoint = doc.body.firstChild;

for (var node in doc.head.nodes.toList(growable: false)) {

if (node is! Element) continue;

- var tag = node.tagName;

+ var tag = node.localName;

var type = node.attributes['type'];

var rel = node.attributes['rel'];

if (tag == 'style' || tag == 'script' &&

@@ -125,7 +128,10 @@ class _HtmlInliner extends PolymerTransformer {

readAsHtml(id, transform).then((doc) => _visitImports(doc, id).then((_) {

new _UrlNormalizer(transform, id).visit(doc);

- _extractScripts(doc);

+ var scripts = doc.querySelectorAll('script');

+ _extractScripts(scripts);

+ _removeScripts(scripts);

// TODO(jmesserly): figure out how this is working in vulcanizer.

// Do they produce a <body> tag with a <head> and <body> inside?

@@ -136,46 +142,84 @@ class _HtmlInliner extends PolymerTransformer {

Future _inlineStylesheet(AssetId id, Element link) {

return transform.readInputAsString(id).then((css) {

- var url = spanUrlFor(id, transform);

- css = new _UrlNormalizer(transform, id).visitCss(css, url);

+ css = new _UrlNormalizer(transform, id).visitCss(css);

link.replaceWith(new Element.tag('style')..text = css);

});

}

- /// Split Dart script tags from all the other elements. Now that Dartium

- /// only allows a single script tag per page, we can't inline script

- /// tags. Instead, we collect the urls of each script tag so we import

- /// them directly from the Dart bootstrap code.

- void _extractScripts(Document document) {

- bool first = true;

- for (var script in document.querySelectorAll('script')) {

+ /// Remove scripts from HTML imports, and remember their [AssetId]s for later

+ /// use.

+ ///

+ /// Dartium only allows a single script tag per page, so we can't inline

+ /// the script tags. Instead we remove them entirely.

+ void _removeScripts(List<Element> scripts) {

+ for (var script in scripts) {

if (script.attributes['type'] == TYPE_DART) {

script.remove();

+ var src = script.attributes['src'];

+ scriptIds.add(resolve(docId, src, logger, script.sourceSpan));

- // only one Dart script per document is supported in Dartium.

- if (first) {

- first = false;

- var src = script.attributes['src'];

- if (src == null) {

- logger.warning('unexpected script without a src url. The '

- 'ImportInliner transformer should run after running the '

- 'InlineCodeExtractor', span: script.sourceSpan);

- continue;

- }

- scriptIds.add(resolve(docId, src, logger, script.sourceSpan));

- } else {

- // TODO(jmesserly): remove this when we are running linter.

- logger.warning('more than one Dart script per HTML '

- 'document is not supported. Script will be ignored.',

- span: script.sourceSpan);

- }

+ // only the first script needs to be added.

+ // others are already removed by _extractInlineScripts

Siggi Cherem (dart-lang) 2014/02/26 22:27:09 _extarctInlineScripts => _extractScripts

Jennifer Messerly 2014/02/27 22:31:40 Done.

+ return;

}

+ /// Split inline scripts into their own files. We need to do this for dart2js

+ /// to be able to compile them.

+ ///

+ /// This also validates that there weren't any duplicate scripts.

+ bool _extractScripts(List<Element> scripts) {

Jennifer Messerly 2014/02/26 03:04:45 the refactored version

+ bool changed = false;

+ bool first = true;

+ int count = 0;

+ for (var script in scripts) {

+ if (script.attributes['type'] != TYPE_DART) continue;

+ // only one Dart script per document is supported in Dartium.

+ if (!first) {

+ // Remove the script. It's invalid to have more than one in Dartium.

+ script.remove();

+ changed = true;

+ // TODO(jmesserly): remove this when we are running linter.

+ logger.warning('more than one Dart script per HTML '

+ 'document is not supported. Script will be ignored.',

+ span: script.sourceSpan);

+ continue;

+ }

+ first = false;

+ var src = script.attributes['src'];

+ if (src != null) continue;

+ var filename = path.url.basename(docId.path);

+ var code = script.text;

+ // TODO(sigmund): ensure this path is unique (dartbug.com/12618).

+ script.attributes['src'] = src = '$filename.$count.dart';

Siggi Cherem (dart-lang) 2014/02/26 22:27:09 I never bothered fixing this, but since there is o

Jennifer Messerly 2014/02/27 22:31:40 good catch! done.

Siggi Cherem (dart-lang) 2014/02/28 01:26:45 sigh - sorry, now that I reread things I think we

Jennifer Messerly 2014/02/28 02:21:49 hah, yeah. Fixed now.

+ script.text = '';

+ changed = true;

+ var newId = docId.addExtension('.$count.dart');

+ if (!_hasLibraryDirective(code)) {

+ // TODO(jmesserly): consolidate this with our other parsing.

+ var libname = path.withoutExtension(newId.path)

+ .replaceAll(new RegExp('[-./]'), '_');

+ code = "library $libname;\n$code";

+ }

+ transform.addOutput(new Asset.fromString(newId, code));

+ }

+ return changed;

+ }

}

+/// Parse [code] and determine whether it has a library directive.

+bool _hasLibraryDirective(String code) =>

+ parseCompilationUnit(code).directives.any((d) => d is LibraryDirective);

/// Recursively inlines the contents of HTML imports. Produces as output a

/// single HTML file that inlines the polymer-element definitions, and a text

/// file that contains, in order, the URIs to each library that sourced in a

@@ -197,6 +241,8 @@ class ImportInliner extends Transformer {

new _HtmlInliner(options, transform).apply();

}

+const TYPE_DART = 'application/dart';

+const TYPE_JS = 'text/javascript';

/// Internally adjusts urls in the html that we are about to inline.

class _UrlNormalizer extends TreeVisitor {

@@ -208,13 +254,20 @@ class _UrlNormalizer extends TreeVisitor {

_UrlNormalizer(this.transform, this.sourceId);

visitElement(Element node) {

- for (var key in node.attributes.keys) {

- if (_urlAttributes.contains(key)) {

- var url = node.attributes[key];

- if (url != null && url != '' && !url.startsWith('{{')) {

- node.attributes[key] = _newUrl(url, node.sourceSpan);

+ node.attributes.forEach((name, value) {

+ if (_urlAttributes.contains(name)) {

+ if (value != '' && !value.trim().startsWith('{{')) {

+ node.attributes[name] = _newUrl(value, node.sourceSpan);

}

+ });

+ if (node.localName == 'style') {

+ node.text = visitCss(node.text);

+ } else if (node.localName == 'script' &&

+ node.attributes['type'] == TYPE_DART) {

+ // TODO(jmesserly): we might need to visit JS too to handle ES Harmony

+ // modules.

+ node.text = visitInlineDart(node.text);

}

super.visitElement(node);

}

@@ -227,7 +280,8 @@ class _UrlNormalizer extends TreeVisitor {

// https://github.com/Polymer/vulcanize/blob/c14f63696797cda18dc3d372b78aa3378acc691f/lib/vulcan.js#L149

// TODO(jmesserly): use csslib here instead? Parsing with RegEx is sadness.

// Maybe it's reliable enough for finding URLs in CSS? I'm not sure.

- String visitCss(String cssText, String url) {

+ String visitCss(String cssText) {

+ var url = spanUrlFor(sourceId, transform);

var src = new SourceFile.text(url, cssText);

return cssText.replaceAllMapped(_URL, (match) {

// Extract the URL, without any surrounding quotes.

@@ -238,7 +292,34 @@ class _UrlNormalizer extends TreeVisitor {

});

}

- _newUrl(String href, Span span) {

+ String visitInlineDart(String code) {

+ var unit = parseCompilationUnit(code);

+ var file = new SourceFile.text(spanUrlFor(sourceId, transform), code);

+ var output = new TextEditTransaction(code, file);

+ for (Directive directive in unit.directives) {

+ if (directive is UriBasedDirective) {

+ var uri = directive.uri.stringValue;

+ var span = _getSpan(file, directive.uri);

+ // TODO(jmesserly): _newUrl supports some things we shouldn't allow

+ // in Dart imports, such as assets/.

+ var targetUri = _newUrl(uri, span);

+ if (targetUri != uri) {

+ output.edit(span.start.offset, span.end.offset, "'$targetUri'");

+ }

+ if (!output.hasEdits) return code;

+ // TODO(sigmund): emit source maps when barback supports it (see

+ // dartbug.com/12340)

+ return (output.commit()..build(file.url)).text;

Siggi Cherem (dart-lang) 2014/02/26 22:27:09 nit: remove empty line

Jennifer Messerly 2014/02/27 22:31:40 eeeeeek. that was ugly! fixed.

+ }

+ String _newUrl(String href, Span span) {

var uri = Uri.parse(href);

if (uri.isAbsolute) return href;

if (!uri.scheme.isEmpty) return href;

@@ -289,3 +370,5 @@ const _urlAttributes = const [

'src', // in audio, embed, iframe, img, input, script, source, track,

// video

];

+_getSpan(SourceFile file, ASTNode node) => file.span(node.offset, node.end);

« no previous file with comments | « pkg/polymer/lib/src/build/common.dart ('k') | pkg/polymer/lib/src/build/script_compactor.dart » ('j') | pkg/polymer/test/build/all_phases_test.dart » ('J')