Repository: Polymer/vulcanize
Pull request #403 (2016-12-08 21:29:37)

Style order preservation
vulcanize was improperly reordering styles because it moved some things out of head but not all things.  I decided the right thing to do was to only move things out of head once an html import is encountered.  I also decided that once an html import is encountered, all subsequent styles, including inlined ones, have to be moved out of head so that style ordering works right. 

<!--
  Thanks for the PR!

  If this change has a user visible change (including
  bug fixes, new features, etc) please describe the change in
  CHANGELOG.md.

  If the change is an entirely package-internal reshuffling/refactoring
  should the change not be described in the CHANGELOG.

  Consider also updating the README.

  More info: http://keepachangelog.com/en/0.3.0/
 -->

 - [x] CHANGELOG.md has been updated
 - Fixed: Preserve style ordering when moving imports and links out of head tag.
 - Don't actually move anything out of head until an html import is encountered.


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4fb22bb..c159dd2 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,9 +7,12 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## Unreleased
 
-<!-- New PRs should document their changes here. -->
+## 1.14.13 - 2016-12-16
 
-## 1.13.12 - 2016-11-16
+- Fixed: Preserve style ordering when moving imports and links out of head tag.
+- Don't actually move anything out of head until an html import is encountered.
+
+## 1.14.12 - 2016-11-16
 
 - Removed update-notifier dependency and functionality
 - Fixed: Stopped moving links with certain rels out of the head tag to avoid breaking
diff --git a/lib/matchers.js b/lib/matchers.js
index ea1fd37..bcbd329 100644
--- a/lib/matchers.js
+++ b/lib/matchers.js
@@ -43,6 +43,15 @@ var polymerExternalStyle = p.AND(
   p.hasAttrValue('type', 'css')
 );
 
+var htmlImport = p.AND(
+  p.hasTagName('link'),
+  p.hasAttrValue('rel', 'import'),
+  p.OR(
+    p.NOT(p.hasAttr('type')),
+    p.hasAttrValue('type', 'text/html')
+  )
+);
+
 var styleMatcher = p.AND(
   p.hasTagName('style'),
   p.OR(
@@ -80,6 +89,7 @@ module.exports = {
   urlAttrs: urlAttrs,
   targetMatcher: targetMatcher,
   polymerExternalStyle: polymerExternalStyle,
+  htmlImport: htmlImport,
   JS: jsMatcher,
   CSS: styleMatcher,
   CSS_LINK: externalStyle,
diff --git a/lib/vulcan.js b/lib/vulcan.js
index 5c4e5e3..9a918f0 100644
--- a/lib/vulcan.js
+++ b/lib/vulcan.js
@@ -177,9 +177,13 @@ Vulcan.prototype = {
   },
 
   moveToBodyMatcher: dom5.predicates.AND(
+    dom5.predicates.NOT(
+        dom5.predicates.parentMatches(
+            dom5.predicates.hasTagName('template'))),
     dom5.predicates.OR(
       dom5.predicates.hasTagName('script'),
-      dom5.predicates.hasTagName('link')
+      dom5.predicates.hasTagName('link'),
+      matchers.CSS
     ),
     dom5.predicates.NOT(
       dom5.predicates.OR(
@@ -215,6 +219,9 @@ Vulcan.prototype = {
     return false;
   },
 
+  isInsideTemplate: dom5.predicates.parentMatches(
+      dom5.predicates.hasTagName('template')),
+
   flatten: function flatten(tree, isMainDoc) {
     var doc = tree.html.ast;
     var imports = tree.imports;
@@ -236,15 +243,28 @@ Vulcan.prototype = {
     } else {
       moveTarget = dom5.constructors.fragment();
     }
-    head.childNodes.filter(this.moveToBodyMatcher).forEach(function(n) {
-      this.removeElementAndNewline(n);
-      dom5.append(moveTarget, n);
+    var htmlImportEncountered = false;
+
+    // Once we encounter an html import, we need to move things into the body,
+    // because html imports contain things that can't be in document
+    // head.
+    dom5.queryAll(head, this.moveToBodyMatcher).forEach(function(n) {
+      if (!htmlImportEncountered && matchers.htmlImport(n)) {
+        htmlImportEncountered = true;
+      }
+      if (htmlImportEncountered) {
+        this.removeElementAndNewline(n);
+        dom5.append(moveTarget, n);
+      }
     }, this);
     this.prepend(body, moveTarget);
     if (imports) {
       for (var i = 0, im, thisImport; i < imports.length; i++) {
         im = imports[i];
         thisImport = importNodes[i];
+        if (this.isInsideTemplate(thisImport)) {
+          continue;
+        }
         if (this.isDuplicateImport(im) || this.isStrippedImport(im)) {
           this.removeElementAndNewline(thisImport);
           continue;
diff --git a/package.json b/package.json
index f60e828..8a77153 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "vulcanize",
-  "version": "1.14.12",
+  "version": "1.14.13",
   "description": "Process Web Components into one output file",
   "main": "lib/vulcan.js",
   "bin": {


Test patch:
diff --git a/test/html/imports/doc-linked-style.css b/test/html/imports/doc-linked-style.css
new file mode 100644
index 0000000..8220d65
--- /dev/null
+++ b/test/html/imports/doc-linked-style.css
@@ -0,0 +1,1 @@
+.from-doc-linked-style { color: blue; }
diff --git a/test/html/imports/import-linked-style.css b/test/html/imports/import-linked-style.css
new file mode 100644
index 0000000..fb146a5
--- /dev/null
+++ b/test/html/imports/import-linked-style.css
@@ -0,0 +1,1 @@
+.from-import-linked-style { color: brown; }
diff --git a/test/html/imports/style-order-test-import.html b/test/html/imports/style-order-test-import.html
new file mode 100644
index 0000000..82711f8
--- /dev/null
+++ b/test/html/imports/style-order-test-import.html
@@ -0,0 +1,4 @@
+<link rel="stylesheet" href="import-linked-style.css">
+<style type="text/css">
+  .from-import-inline-style { color: purple; }
+</style>
diff --git a/test/html/style-order-test-doc.html b/test/html/style-order-test-doc.html
new file mode 100644
index 0000000..e5dbd22
--- /dev/null
+++ b/test/html/style-order-test-doc.html
@@ -0,0 +1,26 @@
+<!DOCTYPE html>
+<!--
+    @license
+    Copyright (c) 2014 The Polymer Project Authors. All rights reserved.
+    This code may only be used under the BSD style license found at http://polymer.github.io/LICENSE.txt
+    The complete set of authors may be found at http://polymer.github.io/AUTHORS.txt
+    The complete set of contributors may be found at http://polymer.github.io/CONTRIBUTORS.txt
+    Code distributed by Google as part of the polymer project is also
+    subject to an additional IP rights grant found at http://polymer.github.io/PATENTS.txt
+-->
+<html lang="en">
+<head>
+  <meta charset="UTF-8">
+  <link rel="stylesheet" href="imports/doc-linked-style.css">
+  <link rel="import" href="imports/style-order-test-import.html">
+  <title>Sample Build</title>
+  <style type="text/css">
+    .from-style-in-doc-head { color: red; }
+  </style>
+</head>
+<body>
+  <style type="text/css">
+    .from-style-in-doc-body { color: green; }
+  </style>
+</body>
+</html>
diff --git a/test/test.js b/test/test.js
index 6a48832..0519484 100644
--- a/test/test.js
+++ b/test/test.js
@@ -1004,6 +1004,27 @@ suite('Vulcan', function() {
       };
       process('test/html/inline-styles.html', callback, options);
     });
+
+    test('Inlined styles have original order preserved', function(done) {
+      var callback = function(err, doc) {
+        if (err) {
+          return done(err);
+        }
+        var styles = dom5.queryAll(doc, dom5.predicates.hasTagName('style'));
+        var rules = styles.map(function(s) {
+          return dom5.serialize(s).trim();
+        }).join('\n').match(/\.[a-z-]+/g);
+        assert.deepEqual(rules, [
+          '.from-doc-linked-style',
+          '.from-import-linked-style',
+          '.from-import-inline-style',
+          '.from-style-in-doc-head',
+          '.from-style-in-doc-body'
+        ]);
+        done();
+      };
+      process('test/html/style-order-test-doc.html', callback, options);
+    });
   });
 
   suite('Add import', function(){