Repository: survivejs/webpack-merge (ts)

Issue:
.smart() cause unwanted loader sequence
as loaders are run with the loaders array ascendingtly, smart merging reverse the config merging order, that would break some loader overload functionality.

Pull request:
Rework loader merging so that it uses append over prepend
This means latter loader definitions will `append` to the existing. This is consistent with webpack evaluation order (bottom to top, right to left).

Closes #31.

Patch:
diff --git a/README.md b/README.md
index 3257da3..b8c5d41 100644
--- a/README.md
+++ b/README.md
@@ -135,9 +135,9 @@ merge.smart({
 {
   loaders: [{
     test: /\.js$/,
-    // prepended because Webpack evaluated these from right to left!
+    // appended because Webpack evaluated these from right to left
     // this way you can specialize behavior and build the loader chain
-    loaders: ['coffee', 'babel']
+    loaders: ['babel', 'coffee']
   }]
 }
 ```
diff --git a/lib/index.js b/lib/index.js
index dcf0e98..bc98956 100644
--- a/lib/index.js
+++ b/lib/index.js
@@ -17,7 +17,15 @@ function mergeLoaders(currentLoaders, newLoaders) {
     })) {
       return mergedLoaders.concat([loader]);
     }
-    return mergedLoaders;
+
+    // Replace query values with newer ones
+    return mergedLoaders.map(function (l) {
+      if (loader.match(loaderNameRe)[0] === l.match(loaderNameRe)[0]) {
+        return loader;
+      }
+
+      return l;
+    });
   }, currentLoaders);
 }
 
@@ -51,14 +59,14 @@ function reduceLoaders(mergedLoaderConfigs, loaderConfig) {
      * properties, concat them
      */
     if (foundLoader.include && !isSameValue(foundLoader.include, loaderConfig.include) || foundLoader.exclude && !isSameValue(foundLoader.exclude, loaderConfig.exclude)) {
-      return mergedLoaderConfigs.concat([loaderConfig]);
+      return [loaderConfig].concat(mergedLoaderConfigs);
     }
 
     // foundLoader.loader is intentionally ignored, because a string loader value should always override
     if (foundLoader.loaders) {
       var newLoaders = loaderConfig.loader ? [loaderConfig.loader] : loaderConfig.loaders || [];
 
-      foundLoader.loaders = mergeLoaders(foundLoader.loaders, newLoaders);
+      foundLoader.loaders = mergeLoaders(newLoaders, foundLoader.loaders);
     }
 
     if (loaderConfig.include) {
@@ -72,7 +80,7 @@ function reduceLoaders(mergedLoaderConfigs, loaderConfig) {
     return mergedLoaderConfigs;
   }
 
-  return mergedLoaderConfigs.concat([loaderConfig]);
+  return [loaderConfig].concat(mergedLoaderConfigs);
 }
 
 function joinArrays(customizer, a, b, key) {
@@ -87,10 +95,6 @@ function joinArrays(customizer, a, b, key) {
       return customResult;
     }
 
-    if (isLoader(key)) {
-      return b.concat(a);
-    }
-
     return a.concat(b);
   }
 
@@ -122,5 +126,5 @@ module.exports.smart = function webpackMerge() {
 };
 
 function isLoader(key) {
-  return ['loaders', 'preLoaders', 'postLoaders'].indexOf(key) >= 0;
+  return ['preLoaders', 'loaders', 'postLoaders'].indexOf(key) >= 0;
 }
\ No newline at end of file
diff --git a/src/index.js b/src/index.js
index 1ba039d..40581d1 100644
--- a/src/index.js
+++ b/src/index.js
@@ -8,10 +8,20 @@ const loaderNameRe = /^([^\?]+)/ig;
 
 function mergeLoaders(currentLoaders, newLoaders) {
   return newLoaders.reduce((mergedLoaders, loader) => {
-    if (mergedLoaders.every(l => loader.match(loaderNameRe)[0] !== l.match(loaderNameRe)[0])) {
+    if (mergedLoaders.every(
+      l => loader.match(loaderNameRe)[0] !== l.match(loaderNameRe)[0])
+    ) {
       return mergedLoaders.concat([loader]);
     }
-    return mergedLoaders;
+
+    // Replace query values with newer ones
+    return mergedLoaders.map(l => {
+      if (loader.match(loaderNameRe)[0] === l.match(loaderNameRe)[0]) {
+        return loader;
+      }
+
+      return l;
+    });
   }, currentLoaders);
 }
 
@@ -41,14 +51,14 @@ function reduceLoaders(mergedLoaderConfigs, loaderConfig) {
      */
     if ((foundLoader.include && !isSameValue(foundLoader.include, loaderConfig.include)) ||
         (foundLoader.exclude && !isSameValue(foundLoader.exclude, loaderConfig.exclude))) {
-      return mergedLoaderConfigs.concat([loaderConfig]);
+      return [loaderConfig].concat(mergedLoaderConfigs);
     }
 
     // foundLoader.loader is intentionally ignored, because a string loader value should always override
     if (foundLoader.loaders) {
       const newLoaders = loaderConfig.loader ? [loaderConfig.loader] : loaderConfig.loaders || [];
 
-      foundLoader.loaders = mergeLoaders(foundLoader.loaders, newLoaders);
+      foundLoader.loaders = mergeLoaders(newLoaders, foundLoader.loaders);
     }
 
     if (loaderConfig.include) {
@@ -62,7 +72,7 @@ function reduceLoaders(mergedLoaderConfigs, loaderConfig) {
     return mergedLoaderConfigs;
   }
 
-  return mergedLoaderConfigs.concat([loaderConfig]);
+  return [loaderConfig].concat(mergedLoaderConfigs);
 }
 
 function joinArrays(customizer, a, b, key) {
@@ -77,10 +87,6 @@ function joinArrays(customizer, a, b, key) {
       return customResult;
     }
 
-    if (isLoader(key)) {
-      return b.concat(a);
-    }
-
     return a.concat(b);
   }
 
@@ -116,5 +122,5 @@ module.exports.smart = function webpackMerge() {
 };
 
 function isLoader(key) {
-  return ['loaders', 'preLoaders', 'postLoaders'].indexOf(key) >= 0;
+  return ['preLoaders', 'loaders', 'postLoaders'].indexOf(key) >= 0;
 }


Test patch:
diff --git a/test.js b/test.js
index 29148b4..052831b 100644
--- a/test.js
+++ b/test.js
@@ -22,7 +22,7 @@ describe('Smart merge', function () {
 });
 
 function normalMergeTests(merge, loadersKey) {
-  it('should prepend recursive structures with ' + loadersKey, function () {
+  it('should append recursive structures with ' + loadersKey, function () {
     const a = {
       module: {}
     };
@@ -47,17 +47,17 @@ function normalMergeTests(merge, loadersKey) {
       module: {}
     };
     result.module[loadersKey] = [{
-      test: /\.css$/,
-      loader: 'b'
-    }, {
-      test: /\.sass$/,
-      loader: 'b'
-    }, {
       test: /\.js$/,
       loader: 'a'
     }, {
       test: /\.jade$/,
       loader: 'a'
+    }, {
+      test: /\.css$/,
+      loader: 'b'
+    }, {
+      test: /\.sass$/,
+      loader: 'b'
     }];
 
     assert.deepEqual(merge(a, b), result);
@@ -79,20 +79,20 @@ function normalMergeTests(merge, loadersKey) {
     }];
     const result = {};
     result[loadersKey] = [{
+      test: /\.js$/,
+      loader: 'a'
+    }, {
       test: /\.js$/,
       loader: 'b'
     }, {
       test: /\.css$/,
       loader: 'b'
-    }, {
-      test: /\.js$/,
-      loader: 'a'
     }];
 
     assert.deepEqual(merge(a, b), result);
   });
 
-  it('should not prepend loaders with ' + loadersKey, function () {
+  it('should not append loaders with ' + loadersKey, function () {
     const a = {};
     a[loadersKey] = [{
       test: /\.js$/,
@@ -108,14 +108,14 @@ function normalMergeTests(merge, loadersKey) {
     }];
     const result = {};
     result[loadersKey] = [{
+      test: /\.js$/,
+      loaders: ['a']
+    }, {
       test: /\.js$/,
       loaders: ['b']
     }, {
       test: /\.css$/,
       loader: 'b'
-    }, {
-      test: /\.js$/,
-      loaders: ['a']
     }];
 
     assert.deepEqual(merge(a, b), result);
@@ -135,10 +135,10 @@ function normalMergeTests(merge, loadersKey) {
     const result = {};
     result[loadersKey] = [{
       test: /\.js$/,
-      loaders: ['a', 'b']
+      loaders: ['a']
     }, {
       test: /\.js$/,
-      loaders: ['a']
+      loaders: ['a', 'b']
     }];
 
     assert.deepEqual(merge(a, b), result);
@@ -163,13 +163,13 @@ function normalMergeTests(merge, loadersKey) {
     const result = {};
     result[loadersKey] = [{
       test: /\.js$/,
-      loaders: ['a', 'b?3']
+      loaders: ['a?1']
     }, {
       test: /\.js$/,
       loaders: ['a?2', 'b']
     }, {
       test: /\.js$/,
-      loaders: ['a?1']
+      loaders: ['a', 'b?3']
     }];
 
     assert.deepEqual(merge(a, b, c), result);
@@ -209,7 +209,7 @@ function smartMergeTests(merge, loadersKey) {
     assert.deepEqual(merge(a, result), result);
   });
 
-  it('should prepend loaders with ' + loadersKey, function () {
+  it('should append loaders with ' + loadersKey, function () {
     const a = {};
     a[loadersKey] = [{
       test: /\.js$/,
@@ -230,7 +230,7 @@ function smartMergeTests(merge, loadersKey) {
       // prepend here!!! this is an exception given normally we want to
       // append instead. without this the loader order doesn't make
       // any sense in this case
-      loaders: ['b', 'a']
+      loaders: ['a', 'b']
     }, {
       test: /\.css$/,
       loader: 'b'
@@ -253,11 +253,7 @@ function smartMergeTests(merge, loadersKey) {
     const result = {};
     result[loadersKey] = [{
       test: /\.js$/,
-      // loaders are evaluated from right to left so it makes sense to
-      // prepend here!!! this is an exception given normally we want to
-      // append instead. without this the loader order doesn't make
-      // any sense in this case
-      loaders: ['ab', 'aa']
+      loaders: ['aa', 'ab']
     }];
 
     assert.deepEqual(merge(a, b), result);
@@ -278,16 +274,14 @@ function smartMergeTests(merge, loadersKey) {
     result[loadersKey] = [{
       test: /\.js$/,
       // loaders are evaluated from right to left so it makes sense to
-      // prepend here!!! this is an exception given normally we want to
-      // append instead. without this the loader order doesn't make
-      // any sense in this case
-      loaders: ['/foo/bar-b.js?c=d', '/foo/bar-a.js?a=b']
+      // append here
+      loaders: ['/foo/bar-a.js?a=b', '/foo/bar-b.js?c=d']
     }];
 
     assert.deepEqual(merge(a, b), result);
   });
 
-  it('should prepend loader and loaders with ' + loadersKey, function () {
+  it('should append loader and loaders with ' + loadersKey, function () {
     const a = {};
     a[loadersKey] = [{
       test: /\.js$/,
@@ -305,10 +299,8 @@ function smartMergeTests(merge, loadersKey) {
     result[loadersKey] = [{
       test: /\.js$/,
       // loaders are evaluated from right to left so it makes sense to
-      // prepend here!!! this is an exception given normally we want to
-      // append instead. without this the loader order doesn't make
-      // any sense in this case
-      loaders: ['b', 'a']
+      // append here so that last one added wins
+      loaders: ['a', 'b']
     }, {
       test: /\.css$/,
       loader: 'b'
@@ -408,7 +400,7 @@ function smartMergeTests(merge, loadersKey) {
     result.module[loadersKey] = [
       {
         test: /\.jsx?$/,
-        loaders: ['isparta-instrumenter', 'eslint']
+        loaders: ['eslint', 'isparta-instrumenter']
       }
     ];
 
@@ -445,11 +437,6 @@ function smartMergeTests(merge, loadersKey) {
       module: {}
     };
     result.module[loadersKey] = [
-      {
-        test: /\.jsx?$/,
-        loaders: ['isparta-instrumenter'],
-        include: 'baz'
-      },
       {
         test: /\.jsx?$/,
         loaders: ['eslint'],
@@ -457,6 +444,11 @@ function smartMergeTests(merge, loadersKey) {
           'foo',
           'bar'
         ]
+      },
+      {
+        test: /\.jsx?$/,
+        loaders: ['isparta-instrumenter'],
+        include: 'baz'
       }
     ];
 
@@ -493,11 +485,6 @@ function smartMergeTests(merge, loadersKey) {
       module: {}
     };
     result.module[loadersKey] = [
-      {
-        test: /\.jsx?$/,
-        loader: 'isparta-instrumenter',
-        include: 'baz'
-      },
       {
         test: /\.jsx?$/,
         loaders: ['eslint'],
@@ -505,6 +492,11 @@ function smartMergeTests(merge, loadersKey) {
           'foo',
           'bar'
         ]
+      },
+      {
+        test: /\.jsx?$/,
+        loader: 'isparta-instrumenter',
+        include: 'baz'
       }
     ];
 
@@ -541,11 +533,6 @@ function smartMergeTests(merge, loadersKey) {
       module: {}
     };
     result.module[loadersKey] = [
-      {
-        test: /\.jsx?$/,
-        loaders: ['isparta-instrumenter'],
-        exclude: 'baz'
-      },
       {
         test: /\.jsx?$/,
         loaders: ['eslint'],
@@ -553,6 +540,11 @@ function smartMergeTests(merge, loadersKey) {
           'foo',
           'bar'
         ]
+      },
+      {
+        test: /\.jsx?$/,
+        loaders: ['isparta-instrumenter'],
+        exclude: 'baz'
       }
     ];
 
@@ -589,11 +581,6 @@ function smartMergeTests(merge, loadersKey) {
       module: {}
     };
     result.module[loadersKey] = [
-      {
-        test: /\.jsx?$/,
-        loader: 'isparta-instrumenter',
-        exclude: 'baz'
-      },
       {
         test: /\.jsx?$/,
         loaders: ['eslint'],
@@ -601,6 +588,11 @@ function smartMergeTests(merge, loadersKey) {
           'foo',
           'bar'
         ]
+      },
+      {
+        test: /\.jsx?$/,
+        loader: 'isparta-instrumenter',
+        exclude: 'baz'
       }
     ];
 
@@ -638,7 +630,7 @@ function smartMergeTests(merge, loadersKey) {
     result.module[loadersKey] = [
       {
         test: /\.js$/,
-        loaders: ['strip?strip[]=debug', 'babel'],
+        loaders: ['babel', 'strip?strip[]=debug'],
         include: [
           'apps',
           'lib',
@@ -736,7 +728,7 @@ function smartMergeIssueTest(merge) {
           {
             test: /\.js$/,
             exclude: /node_modules/,
-            loaders: ['coffee', 'foo', 'babel']
+            loaders: ['babel', 'coffee', 'foo']
           }
         ]
       }
@@ -852,11 +844,11 @@ function mergeTests(merge) {
     };
     const result = {
       loaders: [{
-        test: /\.css$/,
-        loader: 'b'
-      }, {
         test: /\.js$/,
         loader: 'a'
+      }, {
+        test: /\.css$/,
+        loader: 'b'
       }]
     };