Repository: ember-cli/ember-cli
Pull request #6081 (2016-07-15 17:48:31)

[BUGFIX] Header files import concat
This ensures concats are stable, and predictably ordered. This also fixes a prepend bug for test css.

stefanpenner: will fix patch, just a sec.
homu: :umbrella: The latest upstream changes (presumably bb4b51b) made this pull request unmergeable. Please resolve the merge conflicts.
stefanpenner: rebased + retargeted to master.
stefanpenner: Added tests: https://github.com/ember-cli/ember-cli/pull/6081/commits/4528501640c187444e2445bd6cf16cf73bfd2656#diff-1cef190fe22e6c889bb23fda5a8296a3R956

Release: test styles prepend: true, wasn't working and also fixed that.

Patch:
diff --git a/lib/broccoli/ember-app.js b/lib/broccoli/ember-app.js
index 6bf1b052b..bc8f63caa 100644
--- a/lib/broccoli/ember-app.js
+++ b/lib/broccoli/ember-app.js
@@ -1263,11 +1263,11 @@ EmberApp.prototype.javascript = function() {
 
   var vendorFiles = [];
   for (var outputFile in this._scriptOutputFiles) {
-    var inputFiles = this._scriptOutputFiles[outputFile];
+    var headerFiles = this._scriptOutputFiles[outputFile];
 
     vendorFiles.push(
       this._concatFiles(applicationJs, {
-        inputFiles: inputFiles,
+        headerFiles: headerFiles,
         outputFile: outputFile,
         separator: EOL + ';',
         annotation: 'Concat: Vendor ' + outputFile
@@ -1328,10 +1328,10 @@ EmberApp.prototype.styles = function() {
 
   var vendorStyles = [];
   for (var outputFile in this._styleOutputFiles) {
-    var inputFiles = this._styleOutputFiles[outputFile];
+    var headerFiles = this._styleOutputFiles[outputFile];
 
     vendorStyles.push(this._concatFiles(stylesAndVendor, {
-      inputFiles: inputFiles,
+      headerFiles: headerFiles,
       outputFile: outputFile,
       annotation: 'Concat: Vendor Styles' + outputFile
     }));
@@ -1441,7 +1441,7 @@ EmberApp.prototype.testFiles = function(coreTestTree) {
   if (this.vendorTestStaticStyles.length > 0) {
     sourceTrees.push(
       this._concatFiles(external, {
-        inputFiles: this.vendorTestStaticStyles,
+        headerFiles: this.vendorTestStaticStyles,
         outputFile: this.options.outputPaths.testSupport.css,
         annotation: 'Concat: Test Support CSS'
       })
@@ -1538,6 +1538,7 @@ EmberApp.prototype.import = function(asset, options) {
   @param {String} extension
  */
 EmberApp.prototype._import = function(assetPath, options, directory, subdirectory, extension) {
+  // TODO: refactor, this has gotten very messy. Relevant tests: tests/unit/broccoli/ember-app-test.js
   var basename = path.basename(assetPath);
 
   if (isType(assetPath, 'js', {registry: this.registry})) {
@@ -1577,7 +1578,11 @@ EmberApp.prototype._import = function(assetPath, options, directory, subdirector
       options.outputFile = options.outputFile || this.options.outputPaths.vendor.css;
       addOutputFile(this._styleOutputFiles, assetPath, options);
     } else {
-      this.vendorTestStaticStyles.push(assetPath);
+      if (options.prepend) {
+        this.vendorTestStaticStyles.unshift(assetPath);
+      } else {
+        this.vendorTestStaticStyles.push(assetPath);
+      }
     }
   } else {
     var destDir = options.destDir;
diff --git a/package.json b/package.json
index a49355e94..2bc97f1d4 100644
--- a/package.json
+++ b/package.json
@@ -43,7 +43,7 @@
     "broccoli-babel-transpiler": "^5.4.5",
     "broccoli-brocfile-loader": "^0.18.0",
     "broccoli-builder": "^0.18.0",
-    "broccoli-concat": "^2.0.4",
+    "broccoli-concat": "^3.0.2",
     "broccoli-config-loader": "^1.0.0",
     "broccoli-config-replace": "^1.1.2",
     "broccoli-funnel": "^1.0.6",


Test patch:
diff --git a/tests/unit/broccoli/ember-app-test.js b/tests/unit/broccoli/ember-app-test.js
index d1293af06..b8d320636 100644
--- a/tests/unit/broccoli/ember-app-test.js
+++ b/tests/unit/broccoli/ember-app-test.js
@@ -8,6 +8,7 @@ var Project    = require('../../../lib/models/project');
 var expect     = require('chai').expect;
 var proxyquire = require('proxyquire');
 var td = require('testdouble');
+var EOL = require('os').EOL;
 
 var MockUI = require('../../helpers/mock-ui');
 
@@ -938,7 +939,7 @@ describe('broccoli/ember-app', function() {
     });
 
     it('shows deprecation message if called directly', function() {
-      app.concatFiles(null, {
+      app.concatFiles('input-path-somewhere', {
         outputFile: 'foo.js'
       });
 
@@ -946,12 +947,180 @@ describe('broccoli/ember-app', function() {
     });
 
     it('ignores deprecation message if called through _concatFiles()', function() {
-      app._concatFiles(null, {
+      app._concatFiles('input-path-somewhere', {
         outputFile: 'foo.js'
       });
 
       expect(project.ui.output).to.not.contain('EmberApp.concatFiles() is deprecated');
     });
+
+    describe('concat order', function() {
+      var count = 0;
+      var args = [];
+
+      beforeEach(function() {
+        count = 0;
+        args = [];
+
+        // we are "spying and mocking here" so to ensure we are testing our own
+        // "wiring" not the implementation of our dependencies e.g. broccoli-concat
+        app._concatFiles = function(tree, options) {
+          count++
+          args.push(options);
+          return tree;
+        };
+
+        app.appAndDependencies = function() {
+          return 'app-and-dependencies-tree';
+        };
+      });
+
+      it('correctly orders concats from app.styles()', function() {
+        app.import('files/c.css');
+        app.import('files/b.css');
+        app.import('files/a.css', { prepend: true });
+        app.import('files/a.css');
+
+        app.styles(); // run
+
+        expect(count).to.eql(3);
+
+        expect(args[0]).to.deep.eql({
+          allowNone: true,
+          annotation: 'Concat: Addon CSS',
+          inputFiles: [
+            '**/*.css'
+          ],
+          outputFile: '/addons.css'
+        });
+
+        expect(args[1]).to.deep.eql({
+          allowNone: true,
+          annotation: 'Concat: Addon JS',
+          inputFiles: [
+            '**/*.js'
+          ],
+          outputFile: '/addons.js'
+        });
+
+        expect(args[2]).to.deep.eql({
+          annotation: 'Concat: Vendor Styles/assets/vendor.css',
+          headerFiles: [
+            'files/a.css',
+            'files/c.css',
+            'files/b.css',
+            'files/a.css',
+            'vendor/addons.css',
+          ],
+          outputFile: '/assets/vendor.css'
+        });
+      });
+      it('correct orders concats from app.javacsript()', function() {
+        app.import('files/c.js');
+        app.import('files/b.js');
+        app.import('files/a.js', { prepend: true });
+        app.import('files/a.js');
+
+        app.javascript(); // run
+
+        expect(count).to.eql(2);
+        // should be unrelated files
+        expect(args[0]).to.deep.eql({
+          annotation: "Concat: App",
+          footerFiles: [
+            "vendor/ember-cli/app-suffix.js",
+            "vendor/ember-cli/app-config.js",
+            "vendor/ember-cli/app-boot.js"
+          ],
+          headerFiles: [
+            "vendor/ember-cli/app-prefix.js"
+          ],
+          inputFiles: [
+            "test-project/**/*.js"
+          ],
+          outputFile: "/assets/test-project.js"
+        });
+
+        // should be: a,c,b,a in output
+        expect(args[1]).to.deep.eql({
+          annotation: "Concat: Vendor /assets/vendor.js",
+          headerFiles: [
+            "vendor/ember-cli/vendor-prefix.js",
+            "files/a.js",
+            "bower_components/jquery/dist/jquery.js",
+            "bower_components/ember/ember.js",
+            "bower_components/ember-cli-shims/app-shims.js",
+            "files/c.js",
+            "files/b.js",
+            "files/a.js",
+            "vendor/addons.js",
+            "vendor/ember-cli/vendor-suffix.js"
+          ],
+          outputFile: "/assets/vendor.js",
+          separator: EOL + ';',
+        });
+      });
+
+      it('correctly orders concats from app.testFiles()', function() {
+        app.import('files/c.js', { type: 'test'});
+        app.import('files/b.js', { type: 'test'});
+        app.import('files/a.js', { type: 'test', prepend: true });
+        app.import('files/a.js', { type: 'test' });
+
+        app.import('files/c.css', { type: 'test' })
+        app.import('files/b.css', { type: 'test' })
+        app.import('files/a.css', { type: 'test' })
+        app.import('files/a.css', { type: 'test', prepend: true });
+
+        app.testFiles('some-tree'); // run
+
+        expect(count).to.eql(4);
+
+        expect(args[0]).to.deep.eql({
+          allowNone: true,
+          annotation: 'Concat: Addon CSS',
+          inputFiles: ['**/*.css'],
+          outputFile: '/addons.css'
+        });
+
+        expect(args[1]).to.deep.eql({
+          allowNone: true,
+          annotation: 'Concat: Addon JS',
+          inputFiles: ['**/*.js'],
+          outputFile: '/addons.js',
+        });
+
+        expect(args[2]).to.deep.eql({
+          allowNone: true,
+          annotation: 'Concat: Test Support JS',
+          footerFiles: [
+            'vendor/ember-cli/test-support-suffix.js'
+          ],
+          headerFiles: [
+            'vendor/ember-cli/test-support-prefix.js',
+            'files/a.js',
+            'files/c.js',
+            'files/b.js',
+            'files/a.js'
+          ],
+          inputFiles: [
+            'addon-test-support/**/*.js'
+          ],
+          outputFile: '/assets/test-support.js'
+        });
+
+        expect(args[3]).to.deep.eql({
+          annotation: 'Concat: Test Support CSS',
+          headerFiles: [
+            'files/a.css',
+            'files/c.css',
+            'files/b.css',
+            'files/a.css',
+          ],
+          outputFile: '/assets/test-support.css'
+        });
+      });
+    });
   });
 });